On Sat, Apr 22, 2023 at 11:02:23AM +0200, Marc Espie wrote:
> Well, sdk stumbled upon it
> (see docbooks-dsssl-1.79.tgz in snapshots right now)
>
> Turns out that, if the archive is *exactly* a multiple of 64KB,
> we will error out at EOF.
>
> I believe keeping the check for short reads and exiting as
> well for files that do not match 64KB lengths is the right thing
> to do.
>
> (note that this does not affect the security of actual packages
> in any way, since the important verification, namely only passing
> signed checksummed data through the pipe, is preserved)
>
> 0kay ?
And of course, thinking some more, I got a better patch, since
the sha part of the signature should match the file exactly.
So, not only should we exit the loop when we run into a completely
empty buffer, but we should *also* report if we still have checksums
from the gzip header that haven't been matched by any data.
(somewhat unlikely unless done deliberately, because it requires
the file read to be truncated at the checksum divide precisely,
and that won't fall on any natural boundary thanks to the gzip
header length being somewhat arbitrary)
Index: zsig.c
===================================================================
RCS file: /cvs/src/usr.bin/signify/zsig.c,v
retrieving revision 1.18
diff -u -p -r1.18 zsig.c
--- zsig.c 22 Dec 2019 06:37:25 -0000 1.18
+++ zsig.c 22 Apr 2023 09:05:29 -0000
@@ -160,6 +160,8 @@ copy_blocks(int fdout, int fdin, const c
if (more == 0)
break;
}
+ if (n == 0)
+ break;
SHA512_256Data(buffer, n, output);
if (endsha - sha < SHA512_256_DIGEST_STRING_LENGTH-1)
errx(4, "signature truncated");
@@ -172,6 +174,8 @@ copy_blocks(int fdout, int fdin, const c
if (n != bufsize)
break;
}
+ if (endsha != sha)
+ errx(4, "file truncated");
free(buffer);
}
