On Fri, Feb 02, 2024 at 10:09:05AM -0500, Ian McCormack wrote:
> This patch fixes a minor instance of undefined behavior in libdecnumber. It
> was discovered in the Rust bindings for libdecnumber (`dec`) using a custom
> version of MIRI that can execute foreign functions.
>
> Within the function `decFloatFMA`, the pointer `lo->msd` is initialized to
> point to a byte array of size 56.
>
> ```
> uByte acc[FMALEN]; /* for multiplied coefficient in BCD */
> ...
> ub=acc+FMALEN-1; /* where lsd of result will go */
> ...
> lo->msd=ub+1;
> lo->lsd=acc+FMALEN-1;
> ```
> However, `lo->msd` is then offset in increments of 4, which leads to a read
> of two bytes beyond the size of `acc`. This patch switches to reading in
> increments of 2 instead of 4.
>
> Bootstrapped on x86_64-pc-linux-gnu with no regressions.
>
> libdecnumber/ChangeLog
> * decBasic.c: Increment `lo->msd` by 2 instead of 4 in `decFloatFMA`
> to avoid undefined behavior.
Isn't 56 divisible by 4?
Anyway, I think all of
decBasic.c: for (; UBTOUI(umsd)==0 && umsd+3<ulsd;) umsd+=4;
decBasic.c: for (; *umsd==0 && umsd<ulsd;) umsd++;
decBasic.c: for (; UBTOUI(hi->msd)==0 && hi->msd+3<hi->lsd;) hi->msd+=4;
decBasic.c: for (; *hi->msd==0 && hi->msd<hi->lsd;) hi->msd++;
decBasic.c: for (; UBTOUI(lo->msd)==0 && lo->msd+3<lo->lsd;) lo->msd+=4;
decBasic.c: for (; *lo->msd==0 && lo->msd<lo->lsd;) lo->msd++;
decBasic.c: for (; UBTOUI(lo->msd)==0 && lo->msd+3<lo->lsd;) lo->msd+=4;
decBasic.c: for (; *lo->msd==0 && lo->msd<lo->lsd;) lo->msd++;
decCommon.c: for (; *umsd==0 && umsd<ulsd;) umsd++;
look fishy, I'd expect all those conditions should first check if the index
is still in range and only after && read from memory and compare against 0.
I.e. like
for (; umsd+3<ulsd && UBTOUI(umsd)==0;) umsd+=4;
etc.
Does that fix the issue too?
Jakub
