[BusyBox 0000805]: Unterminated buffer in mdev.c

[bugs at busybox.net]

The following issue requires your FEEDBACK. 
====================================================================== 
http://busybox.net/bugs/view.php?id=805 
====================================================================== 
Reported By:                Ken Milmore
Assigned To:                BusyBox
====================================================================== 
Project:                    BusyBox
Issue ID:                   805
Category:                   Other
Reproducibility:            have not tried
Severity:                   minor
Priority:                   normal
Status:                     feedback
====================================================================== 
Date Submitted:             03-28-2006 16:04 PST
Last Modified:              05-07-2006 14:45 PDT
====================================================================== 
Summary:                    Unterminated buffer in mdev.c
Description: 
The buffer used to read the major:minor numbers from sysfs is not correctly
terminated.
======================================================================
Relationships       ID      Summary
----------------------------------------------------------------------
duplicate of        0000651 mdev -s fails to work.
====================================================================== 

---------------------------------------------------------------------- 
 bernhardf - 04-11-06 11:00  
---------------------------------------------------------------------- 
Not sure if i understand you correct, but do you mean something like the
attached, completely untested patchlet (busybox.mdev_devt_parse-fix.diff)
?

Could you please check if putting the '\n' into the sscanf is smaller than
the
temp[--len]=0 which was there before?

TIA. 

---------------------------------------------------------------------- 
 Ken Milmore - 04-11-06 16:08  
---------------------------------------------------------------------- 
What I had in mind was to reinstate the line of code which terminates the
temp buffer with a NUL byte (see inline patch below).  Runnning sscanf on
an unterminated buffer is potentially dangerous.  I do not recommend
adding the newline character to the sscanf template string. As already
discussed, the presence of the newline is a kernel "feature" that it might
be best not to rely upon.
 
--- mdev.c.orig 2006-04-11 23:59:11.000000000 +0100
+++ mdev.c      2006-04-12 00:01:17.000000000 +0100
@@ -48,6 +48,7 @@
        len = read(fd, temp, PATH_MAX-1);
        close(fd);
        if (len < 1) goto end;
+       temp[len] = 0;

        /* Determine device name, type, major and minor */ 

---------------------------------------------------------------------- 
 landley - 05-07-06 14:45  
---------------------------------------------------------------------- 
It seems to work just fine without wasting the extra byte.  In theory, any
trailing garbage is ignored by sscanf anyway.  Are you seeing an actual
failure? 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
03-28-06 16:04  Ken Milmore    New Issue                                    
03-28-06 16:04  Ken Milmore    Status                   new => assigned     
03-28-06 16:04  Ken Milmore    Assigned To               => BusyBox         
04-11-06 11:00  bernhardf      Note Added: 0001260                          
04-11-06 11:00  bernhardf      File Added: busybox.mdev_devt_parse-fix.diff     
              
04-11-06 11:03  bernhardf      Relationship added       duplicate of 0000651
04-11-06 16:08  Ken Milmore    Note Added: 0001266                          
05-07-06 14:45  landley        Note Added: 0001352                          
05-07-06 14:45  landley        Status                   assigned => feedback
======================================================================