Re: [PATCH 10/11] bundle-uri: avoid using undefined output of `sscanf()`

From: Phillip Wood <phillip.wood123@gmail.com>
Date: May 16, 2025, 13:40 UTC
Message-ID: <20250516134054.217202-1-phillip.wood123@gmail.com>
In-Reply-To: <694d8e62-486b-43d4-966c-4fa1e5a95061@crinan.ddns.net>

On 16/05/2025 11:11, Phillip Wood wrote:

> I had a similar thought, though to make sure that we parsed 64 bit 
> values correctly on windows so we'd need something based on strtoumax() 
> I think.

Perhaps something like the diff below which adds strtoul_u64() in a similar vein to strtoul_ui(). I think it's debatable whether we really want to skip leading whitespace so we could perhaps tighten things up by replacing "if (strchr(s, '-'))" with "if (!isdigit(*s))" though that would mean this function would behave slightly differently to strtoul_ui().

Best Wishes

Phillip

---- >8 ----

diff --git a/bundle-uri.c b/bundle-uri.c
index 96d2ba726d9..9dff7a1c09d 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -214,7 +214,7 @@ static int bundle_list_update(const char *key, const char *value,
 	}
 
 	if (!strcmp(subkey, "creationtoken")) {
-		if (sscanf(value, "%"PRIu64, &bundle->creationToken) != 1)
+		if (strtoul_u64(value, 10, &bundle->creationToken))
 			warning(_("could not parse bundle list key %s with value '%s'"),
 				"creationToken", value);
 		return 0;
@@ -533,7 +533,7 @@ static int fetch_bundles_by_token(struct repository *r,
 	if (!repo_config_get_value(r,
 				   "fetch.bundlecreationtoken",
 				   &creationTokenStr) &&
-	    sscanf(creationTokenStr, "%"PRIu64, &maxCreationToken) == 1 &&
+	    strtoul_u64(creationTokenStr,10, &maxCreationToken) &&
 	    bundles.items[0]->creationToken <= maxCreationToken) {
 		free(bundles.items);
 		return 0;
diff --git a/git-compat-util.h b/git-compat-util.h
index 36b9577c8d4..d34d07fce1e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -939,6 +939,22 @@ static inline int strtol_i(char const *s, int base, int *result)
 	return 0;
 }
 
+static inline int strtoul_u64(char const *s, int base, uint64_t *result)
+{
+	uintmax_t ul;
+	char *p;
+
+	errno = 0;
+	/* negative values would be accepted by strtoumax */
+	if (strchr(s, '-'))
+		return -1;
+	ul = strtoumax(s, &p, base);
+	if (errno || *p || p == s || (uint64_t) ul != ul)
+		return -1;
+	*result = ul;
+	return 0;
+}
+
 #ifndef REG_STARTEND
 #error "Git requires REG_STARTEND support. Compile with NO_REGEX=NeedsStartEnd"
 #endif

-- 
2.49.0.897.gfad3eb7d210