[musl] Large overflow in __intscan ignored

[musl] Large overflow in __intscan ignored

[Anuraag Agrawal]

[-- Attachment #1: Type: text/plain, Size: 525 bytes --]

Currently, __intscan, used by functions like strtol, does not seem to check
for overflow during multiplication.

https://git.musl-libc.org/cgit/musl/tree/src/internal/intscan.c#n69

It at the end checks against the limit, e.g. the size of a long

https://git.musl-libc.org/cgit/musl/tree/src/internal/intscan.c#n90

However, if the value overflows and ends up in the range of the limit,
errno will not be set. It seems that each multiplication operation needs to
be checking for overflow and return errno if it ever happens.

[-- Attachment #2: Type: text/html, Size: 831 bytes --]

[musl] Re: Large overflow in __intscan ignored

[Anuraag Agrawal]

[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]

Oops, sorry I should have mentioned, the inputs I am working with that do
not set errno are

999999999999999999999
1000000000000000000000
4790999999999999999999999999999999999999999999999999999999999999999999999999999999999999
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999

For context, they come from test cases in libpg_query

https://github.com/pganalyze/libpg_query/blob/15-latest/test/scan_tests.c#L30

On Fri, Dec 1, 2023 at 2:08 PM Anuraag Agrawal <anuraaga@gmail.com> wrote:

> Currently, __intscan, used by functions like strtol, does not seem to
> check for overflow during multiplication.
>
> https://git.musl-libc.org/cgit/musl/tree/src/internal/intscan.c#n69
>
> It at the end checks against the limit, e.g. the size of a long
>
> https://git.musl-libc.org/cgit/musl/tree/src/internal/intscan.c#n90
>
> However, if the value overflows and ends up in the range of the limit,
> errno will not be set. It seems that each multiplication operation needs to
> be checking for overflow and return errno if it ever happens.
>
>

[-- Attachment #2: Type: text/html, Size: 1884 bytes --]

Re: [musl] Re: Large overflow in __intscan ignored

[Rich Felker]

On Fri, Dec 01, 2023 at 02:14:03PM +0900, Anuraag Agrawal wrote:
> Oops, sorry I should have mentioned, the inputs I am working with that do
> not set errno are
> 
> 999999999999999999999
> 1000000000000000000000
> 4790999999999999999999999999999999999999999999999999999999999999999999999999999999999999
> 9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999
> 
> For context, they come from test cases in libpg_query
> 
> https://github.com/pganalyze/libpg_query/blob/15-latest/test/scan_tests.c#L30
> 
> On Fri, Dec 1, 2023 at 2:08 PM Anuraag Agrawal <anuraaga@gmail.com> wrote:
> 
> > Currently, __intscan, used by functions like strtol, does not seem to
> > check for overflow during multiplication.
> >
> > https://git.musl-libc.org/cgit/musl/tree/src/internal/intscan.c#n69
> >
> > It at the end checks against the limit, e.g. the size of a long
> >
> > https://git.musl-libc.org/cgit/musl/tree/src/internal/intscan.c#n90
> >
> > However, if the value overflows and ends up in the range of the limit,
> > errno will not be set. It seems that each multiplication operation needs to
> > be checking for overflow and return errno if it ever happens.

Can you clarify whether you are reporting a bug in musl or trying to
reuse this code and possibly making breaking changes to your copy? I
cannot reproduce any problem with your test cases. They all correctly
report ERANGE. Checked for strtol, strtoll, and strtoull.

Rich

Re: [musl] Large overflow in __intscan ignored

[Markus Wichmann]

Am Fri, Dec 01, 2023 at 02:08:54PM +0900 schrieb Anuraag Agrawal:
> Currently, __intscan, used by functions like strtol, does not seem to check
> for overflow during multiplication.
>
> https://git.musl-libc.org/cgit/musl/tree/src/internal/intscan.c#n69
>

There is no multiplication in the referenced line. Assuming you mean the
one above it, the loop condition checks that neither the multiplication
nor the addition can overflow. The same holds for the loop on lines 79f.

As far as I can tell, this has been the case since the first version of
this code was checked in in 2012.

Ciao,
Markus

Re: [musl] Large overflow in __intscan ignored

[Anuraag Agrawal]

[-- Attachment #1: Type: text/plain, Size: 920 bytes --]

Hello,

Thanks for checking and very sorry for the noise, I didn't read the loop
condition correctly. I am using Webassembly so the issue must be specific
to it somehow. I will report on wasi-libc to follow up.

Thanks,

On Sat, Dec 2, 2023, 01:21 Markus Wichmann <nullplan@gmx.net> wrote:

> Am Fri, Dec 01, 2023 at 02:08:54PM +0900 schrieb Anuraag Agrawal:
> > Currently, __intscan, used by functions like strtol, does not seem to
> check
> > for overflow during multiplication.
> >
> > https://git.musl-libc.org/cgit/musl/tree/src/internal/intscan.c#n69
> >
>
> There is no multiplication in the referenced line. Assuming you mean the
> one above it, the loop condition checks that neither the multiplication
> nor the addition can overflow. The same holds for the loop on lines 79f.
>
> As far as I can tell, this has been the case since the first version of
> this code was checked in in 2012.
>
> Ciao,
> Markus
>

[-- Attachment #2: Type: text/html, Size: 1462 bytes --]

Re: [musl] Large overflow in __intscan ignored

[Markus Wichmann]

Am Sat, Dec 02, 2023 at 10:42:11AM +0900 schrieb Anuraag Agrawal:
> Hello,
>
> Thanks for checking and very sorry for the noise, I didn't read the loop
> condition correctly. I am using Webassembly so the issue must be specific
> to it somehow. I will report on wasi-libc to follow up.
>
> Thanks,

Don't worry about it. The only way I see this could overflow is if
ULLONG_MAX is not the correct limit for what the compiler implements as
"unsigned long long". Though I don't see how that could be, since musl
defines ULLONG_MAX as the minimum value the type can have according to
the C standard.

Or else, of course, maybe the compiler's implementation of 64-bit
multiplication is broken.

Ciao,
Markus

Re: [musl] Large overflow in __intscan ignored

[Rich Felker]

On Sat, Dec 02, 2023 at 09:48:58AM +0100, Markus Wichmann wrote:
> Am Sat, Dec 02, 2023 at 10:42:11AM +0900 schrieb Anuraag Agrawal:
> > Hello,
> >
> > Thanks for checking and very sorry for the noise, I didn't read the loop
> > condition correctly. I am using Webassembly so the issue must be specific
> > to it somehow. I will report on wasi-libc to follow up.
> >
> > Thanks,
> 
> Don't worry about it. The only way I see this could overflow is if
> ULLONG_MAX is not the correct limit for what the compiler implements as
> "unsigned long long". Though I don't see how that could be, since musl
> defines ULLONG_MAX as the minimum value the type can have according to
> the C standard.
> 
> Or else, of course, maybe the compiler's implementation of 64-bit
> multiplication is broken.

Ouch, sounds like this is going to be some sort of big wasm bug...

Rich

Re: [musl] Large overflow in __intscan ignored

[Rich Felker]

On Sat, Dec 02, 2023 at 11:29:41AM -0500, Rich Felker wrote:
> On Sat, Dec 02, 2023 at 09:48:58AM +0100, Markus Wichmann wrote:
> > Am Sat, Dec 02, 2023 at 10:42:11AM +0900 schrieb Anuraag Agrawal:
> > > Hello,
> > >
> > > Thanks for checking and very sorry for the noise, I didn't read the loop
> > > condition correctly. I am using Webassembly so the issue must be specific
> > > to it somehow. I will report on wasi-libc to follow up.
> > >
> > > Thanks,
> > 
> > Don't worry about it. The only way I see this could overflow is if
> > ULLONG_MAX is not the correct limit for what the compiler implements as
> > "unsigned long long". Though I don't see how that could be, since musl
> > defines ULLONG_MAX as the minimum value the type can have according to
> > the C standard.
> > 
> > Or else, of course, maybe the compiler's implementation of 64-bit
> > multiplication is broken.
> 
> Ouch, sounds like this is going to be some sort of big wasm bug...

Was this bug ever reported/tracked-down/fixed on the wasm side?

Rich