sandflow / regxmllib

Convert MXF to XML: RegXML (SMPTE ST 2001-1) tools and libraries
BSD 2-Clause "Simplified" License
35 stars 14 forks source link

Use of `InputStream.read(byte[] b)` is dangerous if the return value is not checked #70

Closed thomasheritage closed 9 years ago

thomasheritage commented 9 years ago

This sort of construction is used in a few places in the code (in slightly different forms). For example , in applyRule5_2(…, InputStream value, ...):

byte[] val = new byte[len];
value.read(val);
BigInteger bi = idef.isSigned() ? new BigInteger(val) : new BigInteger(1, val);

The number of bytes read from value is not checked. It could be that, for example:

This can lead to some misleading results… For example, if you're expecting an Element to be a UInt16 but it actually contains only a single byte (due to a fault with either the MXF file or with the MetaDefinition) with the value 1 then val will end up as 0x0100 and so the Element will have a value of 256 in the XML output and no Warning or Error will be reported.

This is in contrast to the use of readInt() etc elsewhere which is (probably) safer.

palemieux commented 9 years ago

Fails when the number of bytes read equals 0, otherwise tolerates smaller, but not bigger fields.