Remotely triggerable crash

[Daniel Atallah]

On Fri, Sep 17, 2010 at 11:52, Elliott Sales de Andrade
<qulogic at pidgin.im> wrote:
> On Mon, Sep 13, 2010 at 2:22 PM, Daniel Atallah <datallah at pidgin.im> wrote:
>> I have attached the patch that should fix the places where the API is
>> being misused in the way described above.
>>
>
> I noticed you made changes to QQ and Perl, but didn't write about
> them. Of course, the Perl ones are simple enough to follow. The QQ
> side looks OK too, but I'm not sure it's a good idea to assume the
> decoding will always result in 3 bytes.

Yes, i probably should have mentioned those.  My evaluation was that
those were just "bugs" - not remote crashers.
Good call on the length check for QQ, I'll add that.

>> Based on the fixes in the patch, I believe that the following remotely
>> triggerable NULL pointer dereferences exist:
>> Yahoo:
>>  * An invalid base64 yahoo value related to a buddy icon transfer is
>> received and an uninitialized variable (passed by reference for
>> length) has a non-zero value
>>  * An invalid base64 yahoo value intended to contain the IP address
>> for a P2P connection is received and an uninitialized variable (passed
>> by reference for length) has a non-zero value
>
> There's a swapped comma and space in yahoo_process_p2p here.

Picky, Picky!

>> NTLM: (used for proxy authentication and SIP/SIMPLE authentication)
>>  * An invalid base64 "Type 2" message is received.
>
> Should we also set *flags to a default value if decoding failed?

I thought about that, but it isn't clear to me what to set it to.
I don't think it matters too much - if we don't successfully parse the
nonce, the authentication is certainly going to subsequently fail.

>> XMPP:
>>  * An invalid base64 encoded Digest-MD5 authentication challenge is
>> received (only applies when Cyrus SASL is either unavailable or
>> doesn't provide Digest-MD5 support).
>>
>
> Do we also need to check for dec_in != NULL when printf'ing it or is
> that no longer a problem (I know Windows is OK about it now)?

Newer glib versions don't have a problem with that, but in the
interest of backward compatibility, I'll add that.

Thanks for the feedback.  I've attached an updated patch.

Do we need a CVE for this?

-D
-------------- next part --------------
A non-text attachment was scrubbed...
Name: baseXX_decode_error_handling_2.patch
Type: application/octet-stream
Size: 7289 bytes
Desc: not available
URL: <http://pidgin.im/cgi-bin/mailman/private/security/attachments/20100917/3788b96e/attachment.obj>