[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    kdeplayground-multimedia/daap
From:       Benjamin Meyer <benjamin () csh ! rit ! edu>
Date:       2005-04-24 23:59:38
Message-ID: 20050424235938.95936631 () office ! kde ! org
[Download RAW message or body]

CVS commit by bmeyer: 

Add security problems that must be addresses


  M +40 -0     TODO   1.3


--- kdeplayground-multimedia/daap/TODO  #1.2:1.3
@@ -1,2 +1,42 @@
 -Make server library/deamon that kde application can talk to and share playlists
 -Perhaps give daapclient signals so it doesn't need to be overloaded.
+
+======
+An e-mail from "Aaron J. Seigo" which must be addressed before daap can go into kdemultimedia
+
+i was going over the DAAP implementation that Benjamin Meyer did up for KDE 
+and noticed some issues of concern in the DAAP library he's using
+
+if you've already fixed these issues, please disregard this email. otherwise, 
+these are things that really need to be addressed.
+
+an example of the code that's troubling is to be found in client.c in the 
+DAAP_ClientHost_GetAudioFile function:
+
+    char songUrl_45[] = "daap://%s/databases/%i/items/%i.%s?session-id=%i";
+    char buf[sizeof(songUrl_45) + 11 + 11 + 11 + 11];
+
+first, you'll notice that you're mising space for the integer at the end (%i). 
+there really should be an aditional sizeof(int) there. this is, of course, 4 
+bytes only on 32 bit archs.
+
+then later on in the code you do this:
+
+        sprintf(buf, songUrl_45, pCHThis->host, databaseid, songid,
+                     songformat, pCHThis->sessionid);
+
+
+if it is assumed that each of those items is 10 bytes in length or list, this 
+will work out ok. however, there are no su ch guarantee that i can see in the 
+code. at best this is very brittle code, and at worst this is trivially 
+exploitable. since this library speaks to the network it's obviously of 
+concern.
+
+really what should be done is the buffer should be malloc()'d according to the 
+strlen or sizeof of each of parts that make it up, and then free()'d at the 
+appropriate point.
+
+this style of buffer creation can be seen throughout the DAAP lib and is the 
+sort of thing that would make Theo DeRaadt go into convulsions =)
+
+take care and have a great weekend ...


[prev in list] [next in list] [prev in thread] [next in thread]