bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] progname: don't segfault when argv is NULL


From: Bruno Haible
Subject: Re: [PATCH] progname: don't segfault when argv is NULL
Date: Fri, 4 Dec 2009 23:44:51 +0100
User-agent: KMail/1.9.9

Hi Jim,

> Ok to apply the patch below?
> Without it, anyone can make nearly any coreutils program segfault
> with this simple recipe:
> 
>     printf '%s\n' '#include <unistd.h>' 'int main(int c, char**v)' \
>     '{ execve (v[1], 0, 0); }' > k.c && gcc k.c && ./a.out /bin/cat
> 
> While that usage of execve is in violation of POSIX

One of the purposes of specifications is to avoid redundant checking
of arguments. Imagine, if strlen, strcpy, etc. would have to handle NULL
pointers, how many extra conditional statements a CPU would have to
execute. So, if a spec says that an argument must be non-NULL, it is
the caller's responsibility to not pass NULL, and the callee is freed
from handling NULL.

It makes sense to be "lenient in what you accept", for example when the
spec is unclear, or the violation can easily occur without clear programming
error. But this is not the case here. POSIX's execve spec:
  "The argument arg0 should point to a filename that is associated with
   the process being started by one of the exec functions."

In summary, one can argue for bug-tolerant behaviour, within limits.
This case here is way off-limits.

> nothing prevents a set_program_name caller from calling
> the function with a NULL argument. Hence, we should handle it. 

I disagree. The specs in progname.h say:

  /* Programs using this file should do the following in main():
       set_program_name (argv[0]);
   */

  /* Set program_name, based on argv[0].  */
  extern void set_program_name (const char *argv0);

and together with the POSIX spec this makes it clear that NULL is not
allowed.

But if you want the spec to be more precise, below is a proposal.
Note that the term "a string" already implies non-NULL (see POSIX Base
Definitions, section 3.367).

Bruno


2009-12-04  Bruno Haible  <address@hidden>

        * lib/progname.h (set_program_name): Clarify specification.
        * lib/progname.c (set_program_name): Likewise.

--- lib/progname.c.orig 2009-12-04 23:41:45.000000000 +0100
+++ lib/progname.c      2009-12-04 23:38:19.000000000 +0100
@@ -1,6 +1,6 @@
 /* Program name management.
    Copyright (C) 2001-2003, 2005-2009 Free Software Foundation, Inc.
-   Written by Bruno Haible <address@hidden>, 2001.
+   Written by Bruno Haible <address@hidden>, 2001.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -30,7 +30,9 @@
    To be initialized by main().  */
 const char *program_name = NULL;
 
-/* Set program_name, based on argv[0].  */
+/* Set program_name, based on argv[0].
+   argv0 must be a string allocated with indefinite extent, and must not be
+   modified after this call.  */
 void
 set_program_name (const char *argv0)
 {
--- lib/progname.h.orig 2009-12-04 23:41:45.000000000 +0100
+++ lib/progname.h      2009-12-04 23:37:55.000000000 +0100
@@ -1,6 +1,6 @@
 /* Program name management.
-   Copyright (C) 2001-2004, 2006 Free Software Foundation, Inc.
-   Written by Bruno Haible <address@hidden>, 2001.
+   Copyright (C) 2001-2004, 2006, 2009 Free Software Foundation, Inc.
+   Written by Bruno Haible <address@hidden>, 2001.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -31,7 +31,9 @@
 /* String containing name the program is called with.  */
 extern const char *program_name;
 
-/* Set program_name, based on argv[0].  */
+/* Set program_name, based on argv[0].
+   argv0 must be a string allocated with indefinite extent, and must not be
+   modified after this call.  */
 extern void set_program_name (const char *argv0);
 
 #if ENABLE_RELOCATABLE







reply via email to

[Prev in Thread] Current Thread [Next in Thread]