Bug 12922 - getopt dumps core
Summary: getopt dumps core
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.14
: P2 normal
Target Milestone: ---
Assignee: Ulrich Drepper
URL:
Keywords:
: 13936 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-06-22 17:09 UTC by Eric Blake
Modified: 2014-06-25 11:24 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Blake 2011-06-22 17:09:18 UTC
Detected by Clang:
http://www.gnupdf.org/prmgt/clang/report-kFVhsy.html#EndPath

$ cat foo.c
#include <unistd.h>
int
main (int argc, char **argv)
{
  getopt (argc, argv, "W;");
  return 0;
}
$ ./foo -Wb
Segmentation fault (core dumped)

backtrace:
Program received signal SIGSEGV, Segmentation fault.
0x0000003d15ec7ba8 in _getopt_internal_r (argc=<value optimized out>, 
    argv=0x7fffffffe1c8, optstring=0x400608 "W;", longopts=0x0, longind=0x0, 
    long_only=0, d=0x3d16198460, posixly_correct=1) at getopt.c:899
899		for (p = longopts, option_index = 0; p->name; p++, option_index++)
(gdb) bt
#0  0x0000003d15ec7ba8 in _getopt_internal_r (argc=<value optimized out>, 
    argv=0x7fffffffe1c8, optstring=0x400608 "W;", longopts=0x0, longind=0x0, 
    long_only=0, d=0x3d16198460, posixly_correct=1) at getopt.c:899
#1  0x0000003d15ec844b in _getopt_internal (argc=<value optimized out>, 
    argv=<value optimized out>, optstring=<value optimized out>, 
    longopts=<value optimized out>, longind=<value optimized out>, 
    long_only=<value optimized out>, posixly_correct=1) at getopt.c:1131
#2  0x0000003d15ec84b8 in __posix_getopt (argc=<value optimized out>, 
    argv=<value optimized out>, optstring=<value optimized out>)
    at getopt.c:1155
#3  0x0000000000400509 in main ()

culprit: getopt is blindly assuming a non-NULL long options if the short options string contains the sequence "W;".

Technically, the above program is in violation of POSIX (";" does not have specified behavior as an option character in getopt), but since the crash can also occur with getopt_long, it might make sense to diagnose NULL longopts paired with "W;" as a programming error rather than blindly crashing.
Comment 1 Eric Blake 2011-06-22 17:13:20 UTC
Potential patch:

diff --git i/posix/getopt.c w/posix/getopt.c
index db89abf..da6ce66 100644
--- i/posix/getopt.c
+++ w/posix/getopt.c
@@ -869,7 +869,7 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
 	return '?';
       }
     /* Convenience. Treat POSIX -W foo same as long option --foo */
-    if (temp[0] == 'W' && temp[1] == ';')
+    if (temp[0] == 'W' && temp[1] == ';' && longopts)
       {
 	char *nameend;
 	const struct option *p;
Comment 2 Ulrich Drepper 2011-07-07 01:29:30 UTC
I checked in a patch.
Comment 3 Eric Blake 2011-07-07 14:49:29 UTC
(In reply to comment #2)
> I checked in a patch.

Except that your patch fails to compile with C89 compilers.  Can you also check in this?

diff --git i/posix/getopt.c w/posix/getopt.c
index 3fa5a4d..e0e5503 100644
--- i/posix/getopt.c
+++ w/posix/getopt.c
@@ -871,9 +871,6 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
     /* Convenience. Treat POSIX -W foo same as long option --foo */
     if (temp[0] == 'W' && temp[1] == ';')
       {
-	if (longopts == NULL)
-	  goto no_longs;
-
 	char *nameend;
 	const struct option *p;
 	const struct option *pfound = NULL;
@@ -882,6 +879,9 @@ _getopt_internal_r (int argc, char *const *argv, const char *optstring,
 	int indfound = 0;
 	int option_index;

+	if (longopts == NULL)
+	  goto no_longs;
+
 	/* This is an option that requires an argument.  */
 	if (*d->__nextchar != '\0')
 	  {
Comment 4 Ulrich Drepper 2011-07-08 15:57:20 UTC
(In reply to comment #3)
> Except that your patch fails to compile with C89 compilers.

Who cares?  Stop bothering people with completely irrelevant stuff.
Comment 5 Eric Blake 2011-07-08 16:00:53 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Except that your patch fails to compile with C89 compilers.
> 
> Who cares?

Right now, gnulib and glibc getopt.c are intentionally kept in sync.  This is because _lots_ of projects use getopt.c verbatim in their projects when compiling on non-glibc platforms.  And this includes with C89 compilers.

If you truly don't care about projects that want to port getopt.c outside of glibc, then there is a LOT of cruft that can be removed (look at all the #ifdef _LIBC stuff at the top of the file).  By keeping that portability headache, it implies that you want to keep this file portable, and that (unfortunately) still implies using C89 throughout the file.
Comment 6 Ulrich Drepper 2011-07-08 17:11:05 UTC
C89 compilers are irrelevant.  It's 12 years since C99.  Get over it.
Comment 7 Andreas Schwab 2012-04-02 15:13:24 UTC
*** Bug 13936 has been marked as a duplicate of this bug. ***
Comment 8 Jackie Rosen 2014-02-16 18:27:53 UTC Comment hidden (spam)