[PROPOSED] Don’t assume nonempty argv
Don’t dump core if argv[0] is NULL, which is allowed on GNU/Linux if the invoker is sufficiently perverse. * zdump.c (progname): Now char const *, so that it can be given the address of a string constant. (tzalloc): Use optarg, not progname, since progname’s type is no longer correct. * zdump.c, zic.c (main): Initialize progname to non-null. --- zdump.c | 6 +++--- zic.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/zdump.c b/zdump.c index 512ba8cc..f0461ade 100644 --- a/zdump.c +++ b/zdump.c @@ -84,7 +84,7 @@ static time_t const absolute_max_time = ? (((time_t) 1 << atime_shift) - 1 + ((time_t) 1 << atime_shift)) : -1); static int longest; -static char * progname; +static char const *progname; static bool warned; static bool errout; @@ -234,7 +234,7 @@ tzalloc(char const *val) exit(EXIT_FAILURE); } tzset(); - return &progname; /* Any valid non-null char ** will do. */ + return &optarg; /* Any valid non-null char ** will do. */ # else enum { TZeqlen = 3 }; static char const TZeq[TZeqlen] = "TZ="; @@ -463,7 +463,7 @@ main(int argc, char *argv[]) # endif /* defined TEXTDOMAINDIR */ textdomain(TZ_DOMAIN); #endif /* HAVE_GETTEXT */ - progname = argv[0]; + progname = argv[0] ? argv[0] : "zdump"; for (i = 1; i < argc; ++i) if (strcmp(argv[i], "--version") == 0) { printf("zdump %s%s\n", PKGVERSION, TZVERSION); diff --git a/zic.c b/zic.c index 501718f4..f3b32ecc 100644 --- a/zic.c +++ b/zic.c @@ -943,7 +943,7 @@ main(int argc, char **argv) textdomain(TZ_DOMAIN); #endif /* HAVE_GETTEXT */ main_argv = argv; - progname = argv[0]; + progname = argv[0] ? argv[0] : "zic"; if (TYPE_BIT(zic_t) < 64) { fprintf(stderr, "%s: %s\n", progname, _("wild compilation-time specification of zic_t")); -- 2.37.2
On Fri, Oct 28, 2022 at 11:56 PM Paul Eggert via tz <tz@iana.org> wrote:
Don’t dump core if argv[0] is NULL, which is allowed on GNU/Linux if the invoker is sufficiently perverse.
note that linux fixed this earlier this year: https://github.com/torvalds/linux/commit/dcd46d897adb70d63e025f175a00a89797d...
* zdump.c (progname): Now char const *, so that it can be given the address of a string constant. (tzalloc): Use optarg, not progname, since progname’s type is no longer correct. * zdump.c, zic.c (main): Initialize progname to non-null. --- zdump.c | 6 +++--- zic.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/zdump.c b/zdump.c index 512ba8cc..f0461ade 100644 --- a/zdump.c +++ b/zdump.c @@ -84,7 +84,7 @@ static time_t const absolute_max_time = ? (((time_t) 1 << atime_shift) - 1 + ((time_t) 1 << atime_shift)) : -1); static int longest; -static char * progname; +static char const *progname; static bool warned; static bool errout;
@@ -234,7 +234,7 @@ tzalloc(char const *val) exit(EXIT_FAILURE); } tzset(); - return &progname; /* Any valid non-null char ** will do. */ + return &optarg; /* Any valid non-null char ** will do. */ # else enum { TZeqlen = 3 }; static char const TZeq[TZeqlen] = "TZ="; @@ -463,7 +463,7 @@ main(int argc, char *argv[]) # endif /* defined TEXTDOMAINDIR */ textdomain(TZ_DOMAIN); #endif /* HAVE_GETTEXT */ - progname = argv[0]; + progname = argv[0] ? argv[0] : "zdump"; for (i = 1; i < argc; ++i) if (strcmp(argv[i], "--version") == 0) { printf("zdump %s%s\n", PKGVERSION, TZVERSION); diff --git a/zic.c b/zic.c index 501718f4..f3b32ecc 100644 --- a/zic.c +++ b/zic.c @@ -943,7 +943,7 @@ main(int argc, char **argv) textdomain(TZ_DOMAIN); #endif /* HAVE_GETTEXT */ main_argv = argv; - progname = argv[0]; + progname = argv[0] ? argv[0] : "zic"; if (TYPE_BIT(zic_t) < 64) { fprintf(stderr, "%s: %s\n", progname, _("wild compilation-time specification of zic_t")); -- 2.37.2
On Oct 28, 2022, at 11:56 PM, Paul Eggert via tz <tz@iana.org> wrote:
Don’t dump core if argv[0] is NULL, which is allowed on GNU/Linux if the invoker is sufficiently perverse.
Is argc == 0 in that case? On macOS Monterey, at least, this program: #include <unistd.h> #include <stdio.h> #include <string.h> #include <errno.h> int main(void) { char *args[1] = { NULL }; if (execv("./me", args) == -1) { fprintf(stderr, "Exec of ./me failed: %s\n", strerror(errno)); return 2; } // This cannot possibly happen return 0; } can then exec this program: #include <stdio.h> int main(int argc, char **argv) { printf("argc = %d\n", argc); printf("argv[0] = %p\n", argv[0]); return 0; } and the latter will print argc = 0 argv[0] = 0x0 I suspect at least some other UN*Xes behave in the same fashion. Is the first program a sufficiently perverse invoker, in which case I'd expect that argc == 0 will be true of argv[0] == NULL is true, or are you thinking of something even *more* perverse, that gives you a non-zero argc and a null argv[0]?
Guy Harris wrote in <C690F5F6-E5D4-4EAD-BEFA-3252DF022EE9@sonic.net>: |On Oct 28, 2022, at 11:56 PM, Paul Eggert via tz <tz@iana.org> wrote: |> Don’t dump core if argv[0] is NULL, which is allowed on |> GNU/Linux if the invoker is sufficiently perverse. | |Is argc == 0 in that case? Just to add this was CVE-2021-4034. --steffen | |Der Kragenbaer, The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt)
On Oct 29, 2022, at 1:01 PM, Steffen Nurpmeso <steffen@sdaoden.eu> wrote:
Guy Harris wrote in <C690F5F6-E5D4-4EAD-BEFA-3252DF022EE9@sonic.net>: |On Oct 28, 2022, at 11:56 PM, Paul Eggert via tz <tz@iana.org> wrote: |> Don’t dump core if argv[0] is NULL, which is allowed on |> GNU/Linux if the invoker is sufficiently perverse. | |Is argc == 0 in that case?
Just to add this was CVE-2021-4034.
https://nvd.nist.gov/vuln/detail/cve-2021-4034 "A local privilege escalation vulnerability was found on polkit's pkexec utility. The pkexec application is a setuid tool designed to allow unprivileged users to run commands as privileged users according predefined policies. The current version of pkexec doesn't handle the calling parameters count correctly and ends trying to execute environment variables as commands. An attacker can leverage this by crafting environment variables in such a way it'll induce pkexec to execute arbitrary code. When successfully executed the attack can cause a local privilege escalation given unprivileged users administrative rights on the target machine." See the Qualys blog entry on this: https://blog.qualys.com/vulnerabilities-threat-research/2022/01/25/pwnkit-lo... And a fix for that was committed in... https://gitlab.freedesktop.org/polkit/polkit/-/commit/a2bf5c9c83b6ae46cbd5c7... January 2022. pkexec was parsing the flag arguments with for (n = 1; n < argc; n++) { if (it's a flag argument) process it; else break; } and assuming, afterwards, that argv[n], if non-null, is the first non-flag argument. That meant, if *no* arguments were passed to it, it would go past the end of the array, which stands a good chance of fetching the first element of the array of environment variable settings. The fix was to check, at the beginning of main(), whether argc < 1 and, if so, quitting immediately.
On 2022-10-29 11:55, Guy Harris wrote:
Is the first program a sufficiently perverse invoker, in which case I'd expect that argc == 0 will be true of argv[0] == NULL is true, or are you thinking of something even*more* perverse, that gives you a non-zero argc and a null argv[0]?
Yes, I was thinking about the first case, which is allowed by the C standard and by POSIX. The second case does not conform to C or to POSIX. I don't know of any platforms where it happens. As far as I know, tzcode would work even in this case, though it's merely an accident of the code.
participants (4)
-
enh -
Guy Harris -
Paul Eggert -
Steffen Nurpmeso