diff mbox series

time: Fix use-after-free in getdate

Message ID 20230606172031.2176656-1-arjun@redhat.com
State Committed
Commit 85e6d8b4175fcb195011a0a1bad37d6f3b2355db
Headers
Series time: Fix use-after-free in getdate |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 fail Testing failed
linaro-tcwg-bot/tcwg_glibc_check--master-arm fail Patch failed to apply

Commit Message

Arjun Shankar June 6, 2023, 5:20 p.m. UTC 
  getdate would free the buffer pointed to by the result of its call to
strptime, then reference the same buffer later on -- leading to a
use-after-free.  This commit fixes that.

Reported-by: Martin Coufal <mcoufal@redhat.com>
---
 time/getdate.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Adhemerval Zanella June 6, 2023, 6:47 p.m. UTC | #1 
On 06/06/23 14:20, Arjun Shankar via Libc-alpha wrote:
> getdate would free the buffer pointed to by the result of its call to
> strptime, then reference the same buffer later on -- leading to a
> use-after-free.  This commit fixes that.
> 
> Reported-by: Martin Coufal <mcoufal@redhat.com>

It seems to be introduced by 21f0b087ee10391433d8279e7c6f104fb9ea0eef, and
running time/tst-gettime on valgrind shows the issue.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  time/getdate.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/time/getdate.c b/time/getdate.c
> index 1dcbd77188..ca058394a3 100644
> --- a/time/getdate.c
> +++ b/time/getdate.c
> @@ -114,6 +114,7 @@ __getdate_r (const char *string, struct tm *tp)
>    struct tm tm;
>    struct __stat64_t64 st;
>    bool mday_ok = false;
> +  bool found = false;
>  
>    datemsk = getenv ("DATEMSK");
>    if (datemsk == NULL || *datemsk == '\0')
> @@ -181,7 +182,7 @@ __getdate_r (const char *string, struct tm *tp)
>        tp->tm_gmtoff = 0;
>        tp->tm_zone = NULL;
>        result = strptime (string, line, tp);
> -      if (result && *result == '\0')
> +      if ((found = (result && *result == '\0')))
>  	break;
>      }
>    while (!__feof_unlocked (fp));
> @@ -201,7 +202,7 @@ __getdate_r (const char *string, struct tm *tp)
>    /* Close template file.  */
>    fclose (fp);
>  
> -  if (result == NULL || *result != '\0')
> +  if (!found)
>      return 7;
>  
>    /* Get current time.  */
Andreas Schwab June 6, 2023, 7:04 p.m. UTC | #2 
On Jun 06 2023, Adhemerval Zanella Netto via Libc-alpha wrote:

> It seems to be introduced by 21f0b087ee10391433d8279e7c6f104fb9ea0eef

It already existed before that, just not triggered for typical uses.
diff mbox series

Patch

diff --git a/time/getdate.c b/time/getdate.c
index 1dcbd77188..ca058394a3 100644
--- a/time/getdate.c
+++ b/time/getdate.c
@@ -114,6 +114,7 @@  __getdate_r (const char *string, struct tm *tp)
   struct tm tm;
   struct __stat64_t64 st;
   bool mday_ok = false;
+  bool found = false;
 
   datemsk = getenv ("DATEMSK");
   if (datemsk == NULL || *datemsk == '\0')
@@ -181,7 +182,7 @@  __getdate_r (const char *string, struct tm *tp)
       tp->tm_gmtoff = 0;
       tp->tm_zone = NULL;
       result = strptime (string, line, tp);
-      if (result && *result == '\0')
+      if ((found = (result && *result == '\0')))
 	break;
     }
   while (!__feof_unlocked (fp));
@@ -201,7 +202,7 @@  __getdate_r (const char *string, struct tm *tp)
   /* Close template file.  */
   fclose (fp);
 
-  if (result == NULL || *result != '\0')
+  if (!found)
     return 7;
 
   /* Get current time.  */