[PATCH} Re: I think I found a serious bug in zstrtoul_underscore

[Clinton Bunch <cdbunch@xxxxxxxxxxx>]

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.
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++;