[PATCH v2 04/15] Add code to read and decrypt OSTC3 firmware files

[Jef Driesen]

On 2014-12-17 23:11, Anton Lundin wrote:
> +// This is a variant of fletcher16 with 16 bit sum instead of 8bit
> +// and modulo 256 instead of 255
> +static unsigned int hw_ostc3_firmware_checksum(hw_ostc3_firmware_t 
> *firmware)

The comment is wrong. It's modulo 65536 (2^16) vs 65535 (2^16-1) due to 
the 16bit sum.

> +static dc_status_t
> +hw_ostc3_firmware_readline(FILE *fp, unsigned int addr, unsigned char
> data[], unsigned int size)
> +{
> +	unsigned char ascii[40];
> +	// 1 byte :, 6 bytes addr, X*2 bytes hex -> X bytes data.
> +	const unsigned line_size = size * 2 + 1 + 6 + 1;
> +	unsigned char faddr_byte[3];
> +	unsigned int faddr = 0;
> +	if (line_size > sizeof(ascii))
> +		return DC_STATUS_INVALIDARGS;
> +	if (fread (ascii, sizeof(char), line_size, fp) != line_size)
> +		return DC_STATUS_IO;
> +	if (ascii[0] != ':')
> +		return DC_STATUS_DATAFORMAT;
> +	// Is it a CRLF file?
> +	// Read away that trailing LF
> +	if (ascii[line_size] == '\r')
> +		if (fread (ascii + line_size, sizeof(char), 1, fp) != 1)
> +			return DC_STATUS_DATAFORMAT;
> +	if (array_convert_hex2bin(ascii + 1, 6, faddr_byte,
> sizeof(faddr_byte)) != 0) {
> +		return DC_STATUS_DATAFORMAT;
> +	}
> +	faddr = array_uint24_be (faddr_byte);
> +	if (faddr != addr)
> +		return DC_STATUS_DATAFORMAT;
> +	if (array_convert_hex2bin (ascii + 1 + 6, size*2, data, size) != 0)
> +		return DC_STATUS_DATAFORMAT;
> +
> +	return DC_STATUS_SUCCESS;
> +}

There are two bugs in this code. You read line_size bytes, and then try 
to access ascii[line_size], which is left uninitialized. The ascii array 
is also not large enough to store CRLF, resulting in a buffer overflow.

I have attached a different implementation, which is based on the 
existing ihex code. It can handle arbitrary line endings, and not just 
CRLF or LF. So if you don't mind, I prefer to use this version.

Jef
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-readline.patch
Type: text/x-diff
Size: 4049 bytes
Desc: not available
URL: <http://libdivecomputer.org/pipermail/devel/attachments/20141219/b1a5a141/attachment.patch>