[BusyBox 0004104]: od bug: dangling pointer

[bugs at busybox.net]

A NOTE has been added to this issue. 
====================================================================== 
http://busybox.net/bugs/view.php?id=4104 
====================================================================== 
Reported By:                cristic
Assigned To:                BusyBox
====================================================================== 
Project:                    BusyBox
Issue ID:                   4104
Category:                   Other
Reproducibility:            always
Severity:                   major
Priority:                   normal
Status:                     assigned
====================================================================== 
Date Submitted:             07-14-2008 19:19 PDT
Last Modified:              07-16-2008 14:24 PDT
====================================================================== 
Summary:                    od bug: dangling pointer
Description: 
Hello, I'm part of a research group at Stanford working on automatic
bug finding tools.  We are currently using Busybox as one of our
benchmarks, and I would like to report a series of bugs we found.
Please let me know if this site is not the preferred way of reporting
bugs.

Here is a first bug we found in od: a potential dangling pointer bug,
which can lead to memory corruption.  Here is a very small test case
which exposes the problem:

$ echo -n a >A
$ od A

The problem is in the rewrite() function in dump.c.  Here is the
relevant snippet (dump.c: 234-252, busybox 1.11.1):

(A) pr->fmt = xstrdup(fmtp);
    *p2 = savech;
(B) pr->cchar = pr->fmt + (p1 - fmtp);

   /* DBU:[dave at cray.com] w/o this, trailing fmt text, space is lost.
    * Skip subsequent text and up to the next % sign and tack the
    * additional text onto fmt: eg. if fmt is "%x is a HEX number",
    * we lose the " is a HEX number" part of fmt.
    */
   for (p3 = p2; *p3 && *p3 != '%'; p3++);
   if (p3 > p2)
   {
	savech = *p3;
	*p3 = '\0';
(C)	pr->fmt = xrealloc(pr->fmt, strlen(pr->fmt)+(p3-p2)+1);
	strcat(pr->fmt, p2);
	*p3 = savech;
	p2 = p3;
   }

Line (A) initializes pr->fmt to point to a new buffer.  On line (B),
pr->cchar is also set to point into this buffer.  Later on, pr->fmt is
realloc'ed, potentially pointing to a new region in memory.  However,
pr->cchar is not reset, and it still points to the old buffer. Later
on, when pr->cchar is written into (for example, in bpad(),
dump.c:438), the write can lead to memory corruption, since pr->cchar
may now point to either freed memory or a new buffer which happened to
be allocated there.

Cristian

====================================================================== 

---------------------------------------------------------------------- 
 vda - 07-16-08 00:14  
---------------------------------------------------------------------- 
Your code analyser is cool.

Please try attached 4.patch. 

---------------------------------------------------------------------- 
 cristic - 07-16-08 14:24  
---------------------------------------------------------------------- 
Thanks!  This definitely fixes the dangling pointer issue. 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
07-14-08 19:19  cristic        New Issue                                    
07-14-08 19:19  cristic        Status                   new => assigned     
07-14-08 19:19  cristic        Assigned To               => BusyBox         
07-15-08 18:08  cristic        Issue Monitored: cristic                     
07-16-08 00:13  vda            File Added: 4.patch                          
07-16-08 00:14  vda            Note Added: 0009364                          
07-16-08 14:24  cristic        Note Added: 0009434                          
======================================================================