Re: svn commit: r1704374 - in /subversion/trunk/subversion: include/svn_diff.h include/svn_error_codes.h libsvn_diff/binary_diff.c libsvn_diff/diff.h libsvn_diff/parse-diff.c tests/cmdline/patch_tests.py

Bert Huijben wrote:
> Stefan Sperling wrote:
>> > + unsigned info = 0;
[...]
>> > + for (i = 0; i < 5; i++)
>> > + {
>> > + int value;
>> > +
>> > + SVN_ERR(base85_value(&value, base85_data[i]));
>> > + info *= 85;
>>
>> What happens if 'info' overflows here?
>
> Probably something similar to the git implementation... different final
> output; most likely 100% identical on all systems. Something that would
> happen on every unexpected bad value in a diff file.
>
>> > + info += value;
>> > + }

Just to add a few more words of explanation, for anybody else
wondering about this...

85^5 is 4437053125, a little greater than 256^4 = 2^32 = 4294967296,
so 5 characters of base-85 encoded text is just a little more than
enough to encode 4 bytes of raw data.

'info' needs to be at least 32 bits wide. If it is exactly 32 bits
then it could overflow here if the current group of five base-85
characters is an invalid combination -- or, we could just as well say,
if it is a non-canonical encoding of a smallish 32-bit value. In the
latter interpretation, it will produce an output that we could say is
perfectly valid, a correct decoding of the (non-canonical) input.

Do we care about raising an error in that case? I don't know. It
doesn't seem particularly important to me, one way or the other.

- Julian
Received on 2015-09-22 12:30:58 CEST