On Mon, Jan 13, 2025 at 05:34:27PM +0900, SASANO Takayoshi wrote:
> Hi,
>
> >>>>> The check in explode.c seems correct since it doesn't have this cast.
> >>
> >> I didn't say about cast.
> >
> > But I did...
>
> Yes, I know. No problem.
>
> >> Your diff looks only the fix of inflate.c.
> >> Does explode.c not replace with memmove()? That's my question.
> >
> > You're right It actually does need it. The types involved are unsigned
> > in both files (the cast made me think they weren't) so the overlap check
> > is incorrect in both. Wonderful.
> >
> > What worries me a bit is that the NOMEMCPY path uses this:
> >
> > do {
> > redirSlide[w++] = redirSlide[d++];
> > } while (--e);
> >
> > which isn't handling overlaps the way memmove does if w > d but w - d < e.
>
> Currently I recongnized that the patch will be
>
> - do not use NOMEMCPY
> - replace memcpy() to memmove() within #ifndef NOMEMCPY --- #endif,
> both inflate.c and explode.c
Yes, I would prefer this. The reason is that we only make the current
behavior less incorrect and do not exercise a probably not well tested
code path.
> - suitable cast (or remove?) required
I'd just leave the casts alone. That's for upstream to handle.
> I thought simply replace memcpy() to memmove(), and I didn't consider
> the cast. unzip is highly complicated code so I have no idea how to fix it,
> sorry.
In sump, just replacing the two memcpy with memmove seems the way to go.
If you want to send a diff, I'll ok it, or I can do it.
