[Bug libc/11397] New: calls to cuserid() can result in buffer overflows

[Bug libc/11397] New: calls to cuserid() can result in buffer overflows

[jgeisler at cse dot taylor dot edu]

Since cuserid() uses __getpwuid_r() to obtain the user name and then strncpy()
to copy the user name to the final buffer, the buffer could end up with a
non-NUL terminated value if the login name length >= L_cuserid.  This seems
wrong since L_cuserid is defined as 9 (8 default characters + 1 NUL) for the old
setting when all usernames never exceeded 8 characters.  It seems to me that
since strncpy() is being used, we don't worry about maintaining the final result
when it is too long to fit in a buffer of L_cuserid characters.  Therefore, we
should be safe and free to cut off one additional character to ensure the buffer
truly is NUL-terminated.

As far as I can determine, this bug has existed in glibc for an extremely long
time since this interface is deprecated and probably not actively examined for
this kind of issue.  I only found it due to a legacy app that was crashing on me
when I was logged in with long usernames, but not short usernames.

My suggested patch for sysdeps/posix/cuserid.c is as follows:

--- cuserid.c   2001-12-14 02:00:48.000000000 -0500
+++ cuserid.c.new       2010-03-17 23:08:38.000000000 -0400
@@ -44,5 +44,6 @@
 
   if (s == NULL)
     s = name;
-  return strncpy (s, pwptr->pw_name, L_cuserid);
+  s[L_cuserid - 1] = '\0';
+  return strncpy (s, pwptr->pw_name, L_cuserid - 1);
 }

-- 
           Summary: calls to cuserid() can result in buffer overflows
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: critical
          Priority: P2
         Component: libc
        AssignedTo: drepper at redhat dot com
        ReportedBy: jgeisler at cse dot taylor dot edu
                CC: glibc-bugs at sources dot redhat dot com,jgeisler at cse
                    dot taylor dot edu


http://sourceware.org/bugzilla/show_bug.cgi?id=11397

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/11397] calls to cuserid() can result in buffer overruns and/or overflows

[jgeisler at cse dot taylor dot edu]

------- Additional Comments From jgeisler at cse dot taylor dot edu  2010-03-18 12:14 -------
After a good night's sleep, I realized that the summary line was incorrectly
describing the problem.  cuserid() doesn't cause a buffer overflow, but since it
may not NUL-terminate a C-string, the code that uses the buffer may overrun the
array.  If the calling code isn't careful with size and expects the terminating
NUL (e.g., using strcpy() instead of strncpy()), then buffer overflows can occur.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|calls to cuserid() can      |calls to cuserid() can
                   |result in buffer overflows  |result in buffer overruns
                   |                            |and/or overflows


http://sourceware.org/bugzilla/show_bug.cgi?id=11397

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/11397] calls to cuserid() can result in buffer overruns and/or overflows

[ldv at altlinux dot org]

------- Additional Comments From ldv at altlinux dot org  2010-03-18 13:21 -------
(In reply to comment #0)
> -  return strncpy (s, pwptr->pw_name, L_cuserid);
> +  s[L_cuserid - 1] = '\0';
> +  return strncpy (s, pwptr->pw_name, L_cuserid - 1);

If any change is going to be made for this case, I suggest this one:

-  return strncpy (s, pwptr->pw_name, L_cuserid);
+  s[0] = '\0';
+  return strncat (s, pwptr->pw_name, L_cuserid - 1);



-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11397

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/11397] calls to cuserid() can result in buffer overruns and/or overflows

[jgeisler at cse dot taylor dot edu]

------- Additional Comments From jgeisler at cse dot taylor dot edu  2010-03-18 15:54 -------
That should be the same effect since strncpy() promises to zero fill any unused
portion of the array.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11397

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/11397] calls to cuserid() can result in buffer overruns and/or overflows

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2010-03-24 23:03 -------
The only thing that counts in handling this issue is that the code increase is
kept minimal.  The function should never be used and therefore performance is
irrelevant.  I used the first proposed patch.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


http://sourceware.org/bugzilla/show_bug.cgi?id=11397

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.