Remotely triggerable crash

[Elliott Sales de Andrade]

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.
>

Hi Daniel,

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.

> 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.

> MSN:
>  * An invalid base64 file transfer header is received and an
> uninitialized variable (passed by reference for length) has a value >
> the size of a struct.

Looks good.

> 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?

> MySpace:
>  * An invalid base64 encoded login challenge is received.

Makes sense. I don't know MySpace.

> 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)?

> I'd appreciate it if someone could review the above evaluation and
> make sure that I'm not misunderstanding what could happen.
>
> Thanks,
> -D
>

-- 
Elliott aka QuLogic
Pidgin developer