Re: [PATCH] tree-walk: disallow overflowing modes

From: René Scharfe <l.s.r@web.de>
Date: Jan 22, 2023, 10:03 UTC
Message-ID: <044bdc8f-fdc9-dfd2-6cbb-941513467524@web.de>
In-Reply-To: <Y8zquGar3rLyRdTp@coredump.intra.peff.net>

Am 22.01.23 um 08:50 schrieb Jeff King:

Show 10 quoted lines

> On Sat, Jan 21, 2023 at 10:36:09AM +0100, René Scharfe wrote:
>
>> When parsing tree entries, reject mode values that don't fit into an
>> unsigned int.
>
> Seems reasonable. I don't think you can cause any interesting mischief
> here, but it's cheap to check, and finding data problems earlier rather
> than later is always good.
>
> Should it be s/unsigned int/uint16_t/, though?

"mode" is declared as unsigned int, and I was more concerned with overflowing that.

We could be more strict and reject everything that oversteps S_IFMT|ALLPERMS, but the latter is not defined everywhere. But permission bits are well-known, so the magic number 07777 should be recognizable enough. Like this?

--- >8 ---
Subject: [PATCH v2] tree-walk: disallow overflowing modes

When parsing tree entries, reject mode values with bits set outside file type mask and permission bits.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
 tree-walk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tree-walk.c b/tree-walk.c
index 74f4d710e8..62da0e5c73 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -18,6 +18,8 @@ static const char *get_mode(const char *str, unsigned int *modep)
 		if (c < '0' || c > '7')
 			return NULL;
 		mode = (mode << 3) + (c - '0');
+		if (mode & ~(S_IFMT | 07777))
+			return NULL;
 	}
 	*modep = mode;
 	return str;
--
2.39.1