Re: RFC: cksum --base64/-b support

[Jim Meyering]

On Tue, Jan 31, 2023 at 5:00 AM Pádraig Brady <P@draigbrady.com> wrote:
> On 31/01/2023 06:48, Jim Meyering wrote:
> > On Mon, Jan 30, 2023 at 11:29 AM Pádraig Brady <P@draigbrady.com> wrote:
> >> ...
> >
> > Thanks for the speedy feedback.
> >
> >> "If must be followed by white space." comment has a typo
> >> and also not enforced explicitly, so could be removed.
> >
> > Thanks. Removed.
> >
> >> valid_digits() may check beyond the end of the buffer
> >> in the case len != BASE64_LENGTH.
> >
> > Please share how that could happen.
> > I don't see how there could be a read-overrun there, since the input
> > string is always NUL-terminated.
>
> Oh right I missed that.
> Indeed the !isxdigit() would have terminated upon reaching the NUL.
> Thanks for using the len though as it's easier to read at least.
>
> Similarly I now see that the `while (!ISWHITE())` can write past the NUL 
> terminator,
> which could be triggered with a line full of spaces for example.
> Patch suggested below...

Good catch. An input of just three NUL bytes would trigger a read
buffer overrun:

  printf '\0\0\0' |valgrind src/cksum -a sha1 --check

V3 has a test to cover that.

> > V2 attached.
>
> We should also mention that --check autodetects the encoding.
> I wonder should we have the separate *sum utils autodetect the encoding also,
> but not support the --base64 option. Then `cksum -a blah -b` format
> would be supported by `blahsum -c`. Perhaps it's best to restrict to cksum,
> to aid in the deprecation of the separate utils.

I too prefer to limit this feature addition to cksum.

> So suggested changes...
>
> diff --git a/doc/coreutils.texi b/doc/coreutils.texi

Thanks. Applied along with test addition.

cu-cksum-base64-v3.diff
Description: Binary data