* I think I found a serious bug in zstrtoul_underscore
@ 2022-11-05 21:57 Clinton Bunch
2022-11-05 22:13 ` Clinton Bunch
2022-11-06 8:43 ` Roman Perepelitsa
0 siblings, 2 replies; 7+ messages in thread
From: Clinton Bunch @ 2022-11-05 21:57 UTC (permalink / raw)
To: zsh-workers
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: I think I found a serious bug in zstrtoul_underscore
2022-11-05 21:57 I think I found a serious bug in zstrtoul_underscore Clinton Bunch
@ 2022-11-05 22:13 ` Clinton Bunch
2022-11-05 23:41 ` Clinton Bunch
2022-11-06 8:43 ` Roman Perepelitsa
1 sibling, 1 reply; 7+ messages in thread
From: Clinton Bunch @ 2022-11-05 22:13 UTC (permalink / raw)
To: zsh-workers
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: I think I found a serious bug in zstrtoul_underscore
2022-11-05 22:13 ` Clinton Bunch
@ 2022-11-05 23:41 ` Clinton Bunch
2022-11-06 0:24 ` [PATCH} " Clinton Bunch
0 siblings, 1 reply; 7+ messages in thread
From: Clinton Bunch @ 2022-11-05 23:41 UTC (permalink / raw)
To: zsh-workers
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH} Re: I think I found a serious bug in zstrtoul_underscore
2022-11-05 23:41 ` Clinton Bunch
@ 2022-11-06 0:24 ` Clinton Bunch
2022-11-06 5:16 ` Mikael Magnusson
0 siblings, 1 reply; 7+ messages in thread
From: Clinton Bunch @ 2022-11-06 0:24 UTC (permalink / raw)
To: zsh-workers
[-- 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++;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH} Re: I think I found a serious bug in zstrtoul_underscore
2022-11-06 0:24 ` [PATCH} " Clinton Bunch
@ 2022-11-06 5:16 ` Mikael Magnusson
0 siblings, 0 replies; 7+ messages in thread
From: Mikael Magnusson @ 2022-11-06 5:16 UTC (permalink / raw)
To: Clinton Bunch; +Cc: zsh-workers
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: I think I found a serious bug in zstrtoul_underscore
2022-11-05 21:57 I think I found a serious bug in zstrtoul_underscore Clinton Bunch
2022-11-05 22:13 ` Clinton Bunch
@ 2022-11-06 8:43 ` Roman Perepelitsa
2022-11-06 10:30 ` Clinton Bunch
1 sibling, 1 reply; 7+ messages in thread
From: Roman Perepelitsa @ 2022-11-06 8:43 UTC (permalink / raw)
To: Clinton Bunch; +Cc: zsh-workers
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: I think I found a serious bug in zstrtoul_underscore
2022-11-06 8:43 ` Roman Perepelitsa
@ 2022-11-06 10:30 ` Clinton Bunch
0 siblings, 0 replies; 7+ messages in thread
From: Clinton Bunch @ 2022-11-06 10:30 UTC (permalink / raw)
To: zsh-workers
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.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-06 10:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-05 21:57 I think I found a serious bug in zstrtoul_underscore Clinton Bunch
2022-11-05 22:13 ` Clinton Bunch
2022-11-05 23:41 ` Clinton Bunch
2022-11-06 0:24 ` [PATCH} " Clinton Bunch
2022-11-06 5:16 ` Mikael Magnusson
2022-11-06 8:43 ` Roman Perepelitsa
2022-11-06 10:30 ` Clinton Bunch
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).