From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: fix dubious checksum failures in got-fetch-pack
To: gameoftrees@openbsd.org, Stefan Sperling <stsp@stsp.name>
Date: Sat, 29 Oct 2022 16:31:08 -0600

On October 29, 2022 3:27:34 PM MDT, Stefan Sperling <stsp@stsp.name> wrote:
>Using a work-in-progress regression test suite for gotd, I can trigger
>occasional checksum failures in got-fetch-pack while cloning a repository.
>
>This is a long-standing bug in got-fetch-pack, where only one byte stored
>in the potential sha1 checksum buffer is moved (copied) up, instead of
>shifting all of the remaining bytes up by one position, as was intended. 
>As a result, the checksum we end up computing for the pack file is incorrect.
>
>This can be triggered if the server is doing very short writes, and is
>probably rare enough with git-daemon/Github that we haven't seen it fail
>very often.
>
>ok?

Wow, that's crazy obscure. Ok.

>
>got-fetch-pack: fix wrong memmove length leading to dubious checksum failures
> 
>diff f5d30fbb425287166f3a3ce74eca6adf547bdef7 4fd631967e89f3ee7d5be408e9fe774ff5b2b4d5
>commit - f5d30fbb425287166f3a3ce74eca6adf547bdef7
>commit + 4fd631967e89f3ee7d5be408e9fe774ff5b2b4d5
>blob - b253e5ac03dea40d28e1f2112418a29f0365a7c6
>blob + 10b18441dcd85ef135b54279beb0f68f297be850
>--- libexec/got-fetch-pack/got-fetch-pack.c
>+++ libexec/got-fetch-pack/got-fetch-pack.c
>@@ -688,7 +688,8 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
> 				while (sha1_buf_len > 0 &&
> 				    sha1_buf_len + r > SHA1_DIGEST_LENGTH) {
> 					SHA1Update(&sha1_ctx, sha1_buf, 1);
>-					memmove(sha1_buf, sha1_buf + 1, 1);
>+					memmove(sha1_buf, sha1_buf + 1,
>+					    sha1_buf_len - 1);
> 					sha1_buf_len--;
> 				}
> 
>
>


-- 
Tracey Emery
Sent from my phone.