From: Omar Polo <op@omarpolo.com>
Subject: Re: got-read-patch: don't hardcode SHA1_DIGEST_LENGTH
To: Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Fri, 02 Aug 2024 10:36:47 +0200

On 2024/08/02 08:10:21 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> On Fri, Aug 02, 2024 at 12:49:35AM +0200, Omar Polo wrote:
> > @@ -193,7 +193,11 @@ blobid(const char *line, char **blob, int git)
> >  	if ((*blob = strndup(line, len)) == NULL)
> >  		return got_error_from_errno("strndup");
> >  
> > -	if (!git && !got_parse_hash_digest(digest, *blob, GOT_HASH_SHA1)) {
> > +	if (git)
> > +		return NULL;
> > +
> > +	if (!got_parse_hash_digest(digest, *blob, GOT_HASH_SHA1) &&
> > +	    !got_parse_hash_digest(digest, *blob, GOT_HASH_SHA256)) {
> 
> The order of these checks should be flipped because sha2 is longer.
> A sha2 hash could end up "truncated" and misparsed as a sha1 hash.

That's not important, because digest is then thrown away; what we really
care about is the length of the string, so here's a better diff

diff /home/op/w/got
commit - 13d14aacc10fc9281e7598041152351f23c1592c
path + /home/op/w/got
blob - 9b5c6b818d19c41eb3e9e4ffede13cd4a6fb81f5
file + libexec/got-read-patch/got-read-patch.c
--- libexec/got-read-patch/got-read-patch.c
+++ libexec/got-read-patch/got-read-patch.c
@@ -184,7 +184,6 @@ filexbit(const char *line)
 static const struct got_error *
 blobid(const char *line, char **blob, int git)
 {
-	uint8_t digest[SHA1_DIGEST_LENGTH];
 	size_t len;
 
 	*blob = NULL;
@@ -193,7 +192,11 @@ blobid(const char *line, char **blob, int git)
 	if ((*blob = strndup(line, len)) == NULL)
 		return got_error_from_errno("strndup");
 
-	if (!git && !got_parse_hash_digest(digest, *blob, GOT_HASH_SHA1)) {
+	if (git)
+		return NULL;
+
+	if (len != got_hash_digest_string_length(GOT_HASH_SHA1) -1 &&
+	    len != got_hash_digest_string_length(GOT_HASH_SHA256) -1) {
 		/* silently ignore invalid blob ids */
 		free(*blob);
 		*blob = NULL;