Reporting bugs

[Tomasz Wasilczyk]

2013/8/18 Matt Jones <matt at volvent.org>:

Hi,

thanks for pointing out these flaws. Fortunately, this buggy code will
be dropped in 3.0.0.

>> 1. The potential overflow check [2] is only logged via
>> purple_debug_error and is missing a break; to prevent an actual
>> overflow, this is because purple_debug_error is a debug macro and does
>> not fatally exit or abort.

I'm wondering, why noone hadn't found this missing break before.

>> 2. The check to see if there's a potential overflow [2] is susceptible
>> to an integer overflow due to sz being attacker controlled and not
>> validated at all, meaning the validation check can be bypassed.

I'm not sure, if the < 0 check is necessary. The standard defines
gsize/size_t as unsigned value.

>> 3. There is a potential out of bounds read [2] due to insufficient
>> bounds checking for the CR and LF read.

A check for (*s == '\0') should be enough, am I right?

>>  #define MAX_CHUNK_SIZE 4096

I'm not sure, if 4kB is enough. I've googled about it, and people say
about values like 128kB, or even 5MB. Personally, I think 1MB should
be enough and safe.

I think this issue should be considered confidential and merged just
before the release.

Tomek