[BusyBox 0000939]: dpkg has one buffer overflow and one access to free'd memory

bugs at busybox.net bugs at busybox.net
Mon Sep 11 06:52:21 UTC 2006


A NOTE has been added to this issue. 
====================================================================== 
http://busybox.net/bugs/view.php?id=939 
====================================================================== 
Reported By:                bcg
Assigned To:                BusyBox
====================================================================== 
Project:                    BusyBox
Issue ID:                   939
Category:                   Other
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     assigned
====================================================================== 
Date Submitted:             07-10-2006 04:14 PDT
Last Modified:              09-10-2006 23:52 PDT
====================================================================== 
Summary:                    dpkg has one buffer overflow and one access to
free'd memory
Description: 
SVN 15217 breaks dpkg. It has:

  info_prefix = bb_xasprintf("/var/lib/dpkg/info/%s.", package_name);

...but later on the same function (unpack_package):

  strcat(info_prefix, "list");

Old version was ok:

  info_prefix = (char *) xmalloc(strlen(package_name) + 20 + 4 + 2);
  sprintf(info_prefix, "/var/lib/dpkg/info/%s.", package_name);


There's also another bug in dpkg: If I update from foo-42-1.deb to
foo-42-2.deb, dpkg accesses free'd memory.

Attached is a patch to both bugs. 
====================================================================== 

---------------------------------------------------------------------- 
 vda - 09-07-06 10:20  
---------------------------------------------------------------------- 
strcat() is fixed now.

This hunk seems to be incorrect to me:

        upstream_ver1 = xstrdup(ver1_ptr);
        upstream_ver2 = xstrdup(ver2_ptr);
....

        result = version_compare_part(upstream_ver1, upstream_ver2);
+       if (!result)
+               result = version_compare_part(deb_ver1, deb_ver2);

        free(upstream_ver1);
        free(upstream_ver2);
-
-       if (result != 0) {
-               return(result);
-       }
-
-       /* Compare debian versions */
-       return(version_compare_part(deb_ver1, deb_ver2));
+       return(result);
 }

Why do you think it is correct? It seems to leak upstream_ver1/2... 

---------------------------------------------------------------------- 
 bcg - 09-10-06 23:52  
---------------------------------------------------------------------- 
The old version was something like this:

     upstream_ver1 = xstrdup(ver1_ptr);
...
     result = compare(upstream_ver1, ..
     free(upstream_ver1);
     if (!result) result = compare(upstream_ver1+something, ..

...which clearly accesses upstream_ver1 (via deb_ver1) after free().

My version is:

     upstream_ver1 = xstrdup(ver1_ptr);
...
     result = compare(upstream_ver1, ..
     if (!result) result = compare(upstream_ver1+something, ..
     free(upstream_ver1);

I see no leak here, as I still have free()'s before return. My patch is
just re-ordering the commands, nothing is removed. 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
07-10-06 04:14  bcg            New Issue                                    
07-10-06 04:14  bcg            Status                   new => assigned     
07-10-06 04:14  bcg            Assigned To               => BusyBox         
07-10-06 04:14  bcg            File Added: dpkg.patch                       
09-07-06 10:20  vda            Note Added: 0001624                          
09-10-06 23:52  bcg            Note Added: 0001630                          
======================================================================




More information about the busybox-cvs mailing list