Bug 6435 - modifies line[-1] upon unexpected input (provokes memory corruption)
modifies line[-1] upon unexpected input (provokes memory corruption)
Status: RESOLVED FIXED
Product: Samba 3.0
Classification: Unclassified
Component: File Services
3.0.29
Other Linux
: P3 normal
: none
Assigned To: Volker Lendecke
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-03 12:47 UTC by Jim Meyering
Modified: 2009-11-30 14:51 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jim Meyering 2009-06-03 12:47:05 UTC
Hello,

I've done a partial audit of uses of fgets in 
git://git.samba.org/samba.git and found several cases in which an input of a NUL byte at beginning of line can cause access or even modification of line[-1], which might even result in a segmentation violation.

In fixing these, you'll want to consider whether it is appropriate to discard the "line" following that possible leading NUL byte.  A good alternative is to use getline.  Since it tells you the number of bytes read, it does not have this problem.

I don't know which components might be affected by these uses of fgets, so I have simply listed libsmbclient.  Likewise for the Version.

Regards,

Jim

source3/utils/eventlogadm.c:            if (fgets( linein, sizeof( linein ) - 1, f1 ) == NULL) {
source3/utils/eventlogadm.c-                    break;
source3/utils/eventlogadm.c-            }
source3/utils/eventlogadm.c-            linein[strlen( linein ) - 1] = 0;
---
source3/utils/net_rpc.c-                if (line[strlen(line)-1] == '\n')
source3/utils/net_rpc.c-                        line[strlen(line)-1] = '\0';
---
source3/utils/smbget.c:         if (fgets(tmp, sizeof(tmp), stdin) == NULL) {
source3/utils/smbget.c-                 return;
source3/utils/smbget.c-         }
source3/utils/smbget.c-         if(tmp[strlen(tmp)-1] == '\n')tmp[strlen(tmp)-1] = '\0';
---
source4/torture/nbench/nbench.c:        while (fgets(line, sizeof(line)-1, f)) {
...
source4/torture/nbench/nbench.c-                line[strlen(line)-1] = 0;
Comment 1 Derrell Lipman 2009-10-21 07:57:37 UTC
Select a more appropriate component and reassign to default assignee

Comment 2 Volker Lendecke 2009-11-30 14:51:21 UTC
Applied fixes to master and 3.5. Closing, I'd really call this a minor one. If you need this to be in 3.4 and 3.3, please re-open it.

Thanks,

Volker