Bug 3971 - Some unprotected strncpy() calls
Summary: Some unprotected strncpy() calls
Status: NEW
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: File Services (show other bugs)
Version: 3.0.9
Hardware: All All
: P3 normal
Target Milestone: none
Assignee: Samba Bugzilla Account
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-07-27 15:24 UTC by David S. Collier-Brown
Modified: 2007-09-03 09:05 UTC (History)
1 user (show)

See Also:


Attachments
Diffs to change strncpy(x,y,z-1) to strlcpy(x,y,z) (41.19 KB, patch)
2006-07-27 15:27 UTC, David S. Collier-Brown
no flags Details
Change strncat(x,y,z-1) to strlcat(x,y,z) (5.32 KB, patch)
2006-07-27 15:42 UTC, David S. Collier-Brown
no flags Details
Change sprintf(buf,...) to snprintf(buf, sizeof(buf), ...) (6.33 KB, patch)
2006-07-27 15:54 UTC, David S. Collier-Brown
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description David S. Collier-Brown 2006-07-27 15:24:09 UTC
Many but not all calls to strncpy check and stuff in a
null at the end of the strings, but a number do not,
which can cause strings NOT to be null-terminatd. 
This causes mysterious errors, usually sigsegv (;-))

  This was found by Dieter Jurzitza, initially in 
pam/winbind.c,

  Patches to use strlcpy (the new standard intrface) and strlcat 
follow. 

--dave
Comment 1 David S. Collier-Brown 2006-07-27 15:27:21 UTC
Created attachment 2070 [details]
Diffs to change strncpy(x,y,z-1) to strlcpy(x,y,z)

Patch one of three, the strncpy calls. To be followed
by strncat->strlcate and optionally sprintf->snprintf.
Comment 2 David S. Collier-Brown 2006-07-27 15:42:05 UTC
Created attachment 2071 [details]
Change strncat(x,y,z-1) to strlcat(x,y,z)

Patch 2 of three, strncat.
Comment 3 David S. Collier-Brown 2006-07-27 15:54:25 UTC
Created attachment 2072 [details]
Change sprintf(buf,...) to snprintf(buf, sizeof(buf), ...)

And the sprintfs where the write didn't
preallocate the exact right size...
Comment 4 David S. Collier-Brown 2006-08-01 09:29:27 UTC
The obligatory caveat: This set of patches should be
applied to a test machine and the full regression test
run, as there is room for a mechanical error on my part
to over-truncate strings being built up from sequences
of strlcpy/strlcat.

I was careful, but that's necessary, not sufficient (;-))

--dave
Comment 5 David S. Collier-Brown 2007-07-14 16:58:10 UTC
The same code appears in samba 4: 
  strncpy(a,b,n); a[n+1] = NULL;
Any time someone misses the null, or gets the offset wrong, 
one gets hard-to-diagnose bugs and runtime crashes.

Should this be submitted as a samba4 bug?

--dave
Comment 6 David S. Collier-Brown 2007-09-03 09:05:25 UTC
This fix is now out of date: I'll rescan for erronious strncpys, which
can trivially cause buffer overruns. This was first encounte red by Dieter
Jurzitza, in pam/winbind.c

--dave