daap (itunes music sharing)

[Aaron J. Seigo]

On Monday 28 February 2005 06:19, Benjamin Meyer wrote:
> This evening I added to kdeplayground-multimedia a daap directory.  :) 

cool stuff... someone on irc asked for some help with it because Task was 
polluting the namespace... find a patch here:

	http://aseigo.bddf.ca/dms/11/147_libdaapclient.diff

which fixes this.

i'm a bit concerned that this requires subclassing to use.. that's always a 
bit lame when we have signals/slots around... the switch to making the 
protected virtual methods signals instead was trivial. there are probably 
some style issues, like:

	newTrackData( &file, track );

why pass a pointer around instead of a reference? those are details though...

what i'm really concerned about is that this library talks to other machines 
on the network, but then has code like this (slightly reformatted for email):

=======
    char songUrl_45[] = "daap://%s/databases/%i/items/%i.%s?session-id=%i";

    char requestid_45[] = "Client-DAAP-Request-ID: %u\r\n";

    GetFile *pGetFile = malloc(sizeof(GetFile) + (sizeof(songUrl_45) + 11 + 11 
+ 11 + 11) + (sizeof(requestid_45) + 11));

*snip .... *

   sprintf(pGetFile->url, songUrl_45, pCHThis->host, databaseid, songid,
                songformat, pCHThis->sessionid);
=======

there are so many things wrong this... pCHThis->host is a char* but 11 bytes 
are magically added for it.. hopefully this never gets set to anything larger 
than 13 bytes (11 + the %s it replaces)... the author also forgot to add 4 
bytes for the last %i (though he caught that elsewhere)... 

this just jibba jabba is just begging for an exploit.

it should obviously be using null safe calls to strlen and using sizeof(int), 
etc...  this doesn't fill me with confidence for this library. it's not a 
trivial library, with it's use of multiple threads, networking, and C 
primitives like char* to store data. if my brief glance into it to see how 
things worked turned up problems like the above ...... 

i wouldn't feel comfortable with this being in KDE's CVS without a good 
security audit done on it first.

-- 
Aaron J. Seigo
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-multimedia/attachments/20050309/ab8c451e/attachment.sig>
-------------- next part --------------
_______________________________________________
kde-multimedia mailing list
kde-multimedia at kde.org
https://mail.kde.org/mailman/listinfo/kde-multimedia