This is how I'm calling it: if (!zstrtoul_underscore(OPT_ARG(ops, 'L'), &lower)) { lower is declared as a zulong. When fed a negative number it seg faults, on Rocky Linux 8 at least. Example: helikon[~/tmp/zsh]% getrandom -L 0 -U $(( 2**32-1)) Test before convert -L Test after convert -L 377689410 helikon[~/tmp/zsh]% getrandom -L -1 -U 5 Test before convert -L zsh: segmentation fault (core dumped) ~/local/bin/zsh
On 11/5/2022 4:57 PM, Clinton Bunch wrote:
> This is how I'm calling it:
>
> if (!zstrtoul_underscore(OPT_ARG(ops, 'L'), &lower)) {
>
> lower is declared as a zulong.
>
> When fed a negative number it seg faults, on Rocky Linux 8 at least.
>
> Example:
>
> helikon[~/tmp/zsh]% getrandom -L 0 -U $(( 2**32-1))
> Test before convert -L
> Test after convert -L
> 377689410
> helikon[~/tmp/zsh]% getrandom -L -1 -U 5
> Test before convert -L
> zsh: segmentation fault (core dumped) ~/local/bin/zsh
>
>
I've verified this on FreeBSD 13.1 and Solaris 11.4 as well, so unless
I'm calling it wrong, this seems like a serious bug.
On 11/5/2022 5:13 PM, Clinton Bunch wrote:
> On 11/5/2022 4:57 PM, Clinton Bunch wrote:
>> This is how I'm calling it:
>>
>> if (!zstrtoul_underscore(OPT_ARG(ops, 'L'), &lower)) {
>>
> I've verified this on FreeBSD 13.1 and Solaris 11.4 as well, so unless
> I'm calling it wrong, this seems like a serious bug.
>
>
Apparently the problem is with argument handling. Zsh appears to
interpret a negative number as another option (but doesn't error with
unknown option) but I get the same seg-fault when I don't specify an
argument to -L as when I specify a negative number.
OPT_ARG_SAFE seems to prevent the seg-fault at that point, but I still
get one in zstrtoul_underscore. It seems like specifying that -L
accepts an argument (specifically a number) should cause it to throw an
error at argument parse time rather than at recovering the argument.
[-- Attachment #1: Type: text/plain, Size: 1470 bytes --] On 11/5/2022 6:41 PM, Clinton Bunch wrote: > On 11/5/2022 5:13 PM, Clinton Bunch wrote: >> On 11/5/2022 4:57 PM, Clinton Bunch wrote: >>> This is how I'm calling it: >>> >>> if (!zstrtoul_underscore(OPT_ARG(ops, 'L'), &lower)) { >>> >> I've verified this on FreeBSD 13.1 and Solaris 11.4 as well, so >> unless I'm calling it wrong, this seems like a serious bug. >> >> > Apparently the problem is with argument handling. Zsh appears to > interpret a negative number as another option (but doesn't error with > unknown option) but I get the same seg-fault when I don't specify an > argument to -L as when I specify a negative number. > > OPT_ARG_SAFE seems to prevent the seg-fault at that point, but I still > get one in zstrtoul_underscore. It seems like specifying that -L > accepts an argument (specifically a number) should cause it to throw > an error at argument parse time rather than at recovering the argument. > > I see now my mistake in the options segment of bintab, ":%" specifies an *optional* numeric argument. Removing the % makes it fail correctly. Which leads to the question why would there be an optional numeric argument specification, but not a mandatory numeric argument specification? Though it did uncover a pathological case that caused a catastrophic failure in zstrtoul_underscore. It doesn't check that s is null before it starts trying to convert. I've attached the trivial patch. [-- Attachment #2: zstrtoul_underscore-NULL.patch --] [-- Type: text/plain, Size: 284 bytes --] diff --git a/Src/utils.c b/Src/utils.c index edf5d3df7..ede7b7af6 100644 --- a/Src/utils.c +++ b/Src/utils.c @@ -2481,6 +2481,9 @@ zstrtoul_underscore(const char *s, zulong *retval) { zulong calc = 0, newcalc = 0, base; + if (!s) + return 0; + if (*s == '+') s++;
On 11/6/22, Clinton Bunch <cdbunch@zentaur.org> wrote:
> I see now my mistake in the options segment of bintab, ":%" specifies an
> *optional* numeric argument. Removing the % makes it fail correctly.
> Which leads to the question why would there be an optional numeric
> argument specification, but not a mandatory numeric argument specification?
>
> Though it did uncover a pathological case that caused a catastrophic
> failure in zstrtoul_underscore. It doesn't check that s is null before
> it starts trying to convert. I've attached the trivial patch.
If we did this, it would make more sense to add the check in
zstrtol_underscore which has more than one current user, however it is
expected that functions like this crash on NULL, the standard library
strtol (and most other str*) does this as well.
--
Mikael Magnusson
On Sat, Nov 5, 2022 at 10:58 PM Clinton Bunch <cdb_zsh@zentaur.org> wrote:
>
>
> helikon[~/tmp/zsh]% getrandom -L 0 -U $(( 2**32-1))
What is getrandom?
Roman.
On 11/6/2022 2:43 AM, Roman Perepelitsa wrote: > On Sat, Nov 5, 2022 at 10:58 PM Clinton Bunch <cdb_zsh@zentaur.org> wrote: >> >> helikon[~/tmp/zsh]% getrandom -L 0 -U $(( 2**32-1)) > What is getrandom? It's a builtin in the zsh/random module I'm writing. It gets random data from arc4random or getrandom and falls back to /dev/urandom if those aren't available. > > Roman. >