[Skiboot] [PATCH] libstb: Fix memcpy overread in fakenv_readpublic() - is truncation correct?

[Kenneth Goldman]

> From: Reza Arbab <arbab at linux.ibm.com>
> Sent: Wednesday, May 25, 2022 5:17 PM
> To: Kenneth Goldman <kgoldman at us.ibm.com>
> Cc: skiboot at lists.ozlabs.org
> Subject: Re: [EXTERNAL] [Skiboot] [PATCH] libstb: Fix memcpy overread in
> fakenv_readpublic() - is truncation correct?
> 
> On Wed, May 25, 2022 at 08:56:49PM +0000, Kenneth Goldman wrote:
> >[kgold] Is truncating the Name the right solution?
> >
> >I would have thought that, if the source length is greater than the
> >target length, an error results.  The size of the target should be that
> >of the largest hash algorithm.  If a Name is larger, didn't something
> >go badly wrong?
> 
> It's actually the opposite. The source buffer is 34 bytes and the
> destination buffer is 132 bytes. The bug is that we're using the
> destination buffer's size as the argument to memcpy, which will result
> in an overread of the source.
[kgold] 
1. If the buffers are both fixed sizes, why the MIN function.  It will
compile down to 34.
2. I wonder why 34, not 32.  If there a uint16_t size that's being
copied but not checked?
3. If the source is larger, it gets truncated.
4. This might be tricky enough that comments for the maintainer
are welcome.  (Just my coding style.)