Sun Apr 21 08:56:49 2024 UTC (18d)

make: fix out-of-bounds read when evaluating :gmtime and :localtime

The function TryParseTime takes a pointer to a string, but the LazyBuf
returns a Substring, which is not guaranteed to be null-terminated or
delimited.  In TryParseTime, calling strtoul on the Substring read past
the end of the substring.

Noticed in the NetBSD build in libntp, where the :gmtime modifier is
used in two places with the same timestamp value, of which the first was
evaluated correctly and the second wasn't.

The bug was introduced in var.c 1.1050 from 2023-05-09, when the
argument of the :gmtime and :localtime modifiers was allowed to be an
expression instead of an integer constant.


(rillig)

diff -r1.1102 -r1.1103 src/usr.bin/make/var.c

--- src/usr.bin/make/var.c 2024/04/20 10:18:55	1.1102
+++ src/usr.bin/make/var.c 2024/04/21 08:56:49	1.1103

 @@ -1,14 +1,14 @@
-/*	$NetBSD: var.c,v 1.1102 2024/04/20 10:18:55 rillig Exp $	*/
+/*	$NetBSD: var.c,v 1.1103 2024/04/21 08:56:49 rillig Exp $	*/
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
  *	The Regents of the University of California.  All rights reserved.
+ *
  * This code is derived from software contributed to Berkeley by
  * Adam de Boor.
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
 @@ -127,27 +127,27 @@
 #include <sys/types.h>
 #include <regex.h>
 #include <errno.h>
 #include <inttypes.h>
 #include <limits.h>
 #include <time.h>
 #include "make.h"
 #include "dir.h"
 #include "job.h"
 #include "metachar.h"
 /*	"@(#)var.c	8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: var.c,v 1.1102 2024/04/20 10:18:55 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.1103 2024/04/21 08:56:49 rillig Exp $");
 /*
  * Variables are defined using one of the VAR=value assignments.  Their
  * value can be queried by expressions such as $V, ${VAR}, or with modifiers
  * such as ${VAR:S,from,to,g:Q}.
+ *
  * There are 3 kinds of variables: scope variables, environment variables,
  * undefined variables.
+ *
  * Scope variables are stored in a GNode.scope.  The only way to undefine
  * a scope variable is using the .undef directive.  In particular, it must
  * not be possible to undefine a variable during the evaluation of an
  * expression, or Var.name might point nowhere.  (There is another,
 @@ -2505,50 +2505,69 @@ TryParseTime(const char **pp, time_t *ou
 	if (!ch_isdigit(**pp))
 		return false;
 	errno = 0;
 	n = strtoul(*pp, &end, 10);
 	if (n == ULONG_MAX && errno == ERANGE)
 		return false;
 	*pp = end;
 	*out_time = (time_t)n;	/* ignore possible truncation for now */
 	return true;
+}
 static bool
 Substring_ParseTime(Substring s, time_t *out_time)
+{
 	const char *p;
 	unsigned long n;
 	n = 0;
 	for (p = s.start; p != s.end && ch_isdigit(*p); p++) {
 		unsigned long next = 10 * n + ((unsigned)*p - '0');
 		if (next < n)
 			return false;
 		n = next;
+	}
 	if (p == s.start || p != s.end)
 		return false;
 	*out_time = (time_t)n;	/* ignore possible truncation for now */
 	return true;
+}
 /* :gmtime and :localtime */
 static ApplyModifierResult
 ApplyModifier_Time(const char **pp, ModChain *ch)
+{
 	Expr *expr;
 	time_t t;
 	const char *args;
 	const char *mod = *pp;
 	bool gmt = mod[0] == 'g';
 	if (!ModMatchEq(mod, gmt ? "gmtime" : "localtime", ch))
 		return AMR_UNKNOWN;
 	args = mod + (gmt ? 6 : 9);
 	if (args[0] == '=') {
 		const char *p = args + 1;
 		LazyBuf buf;
 		if (!ParseModifierPartSubst(&p, true, '\0', ch->expr->emode,
 		    ch, &buf, NULL, NULL))
 			return AMR_CLEANUP;
 		if (ModChain_ShouldEval(ch)) {
 			Substring arg = LazyBuf_Get(&buf);
-			const char *arg_p = arg.start;
+			if (!Substring_ParseTime(arg, &t)) {
 			if (!TryParseTime(&arg_p, &t) || arg_p != arg.end) {
 				Parse_Error(PARSE_FATAL,
 				    "Invalid time value \"%.*s\"",
 				    (int)Substring_Length(arg), arg.start);
 				LazyBuf_Done(&buf);
 				return AMR_CLEANUP;
+			}
 		} else
 			t = 0;
 		LazyBuf_Done(&buf);
 		*pp = p;
 	} else {
 		t = 0;
 		*pp = args;