touch pseudo-buglet

[Jim Meyering]

Coverity complained that upon localtime failure, a NULL "tm" might
be dereferenced here:

           struct tm const *tm = localtime (&newtime[0].tv_sec);
-          error (0, 0,
-                 _("warning: `touch %s' is obsolete; use "
-                   "`touch -t %04ld%02d%02d%02d%02d.%02d'"),
-                 argv[optind],
-                 tm->tm_year + 1900L, tm->tm_mon + 1, tm->tm_mday,
-                 tm->tm_hour, tm->tm_min, tm->tm_sec);

However, all of my attempts to provoke such a dereference
have been in vain ;-)

    $ zdump -v US/Mountain|grep -B1 '1999.*03:00'
    US/Mountain  Sun Apr  4 08:59:59 1999 UTC = Sun Apr  4 01:59:59 1999 MST 
isdst=0 gmtoff=-25200
    US/Mountain  Sun Apr  4 09:00:00 1999 UTC = Sun Apr  4 03:00:00 1999 MDT 
isdst=1 gmtoff=-21600
    $ TZ=US/Mountain date -d '1999-04-04 02:30'
    date: invalid date `1999-04-04 02:30'
    [Exit 1]
    $ _POSIX2_VERSION=0 ./touch 0404023099 k
    ./touch: warning: `touch 0404023099' is obsolete; use `touch -t 
199904040230.00'
    $ TZ=US/Mountain _POSIX2_VERSION=0 ./touch 0404023099 k

The reason is that to reach the vulnerable code,
the offending date string must first pass through the
preceding posixtime function.  However, that function
just happens to validate the resulting date using...
you guessed it: localtime.

I've added the guard nonetheless, so that we won't have
to investigate this again the next time a static analysis
program dings that code.


>From 5ed668653f3b0c931325d9cc3b5e88f5cf1125ff Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 24 May 2011 20:33:27 +0200
Subject: [PATCH] touch: placate static analyzers: no NULL-deref is possible

* src/touch.c (main): Avoid even the hint of possibility that
we'd dereference NULL upon localtime failure.  Coverity reported
the potential, but it appears not to be possible, since posixtime
rejects any time for which the subsequent localtime would return NULL.
---
 src/touch.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/touch.c b/src/touch.c
index 49be961..18f81c8 100644
--- a/src/touch.c
+++ b/src/touch.c
@@ -405,12 +405,18 @@ main (int argc, char **argv)
       if (! getenv ("POSIXLY_CORRECT"))
         {
           struct tm const *tm = localtime (&newtime[0].tv_sec);
-          error (0, 0,
-                 _("warning: `touch %s' is obsolete; use "
-                   "`touch -t %04ld%02d%02d%02d%02d.%02d'"),
-                 argv[optind],
-                 tm->tm_year + 1900L, tm->tm_mon + 1, tm->tm_mday,
-                 tm->tm_hour, tm->tm_min, tm->tm_sec);
+
+          /* Technically, it appears that even a deliberate attempt to cause
+             the above localtime to return NULL will always fail because our
+             posixtime implementation rejects all dates for which localtime
+             would fail.  However, skip the warning if it ever fails.  */
+          if (tm)
+            error (0, 0,
+                   _("warning: `touch %s' is obsolete; use "
+                     "`touch -t %04ld%02d%02d%02d%02d.%02d'"),
+                   argv[optind],
+                   tm->tm_year + 1900L, tm->tm_mon + 1, tm->tm_mday,
+                   tm->tm_hour, tm->tm_min, tm->tm_sec);
         }

       optind++;
--
1.7.5.2.585.gfbd48