Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

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



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


Messages sorted by: Reverse Date, Date, Thread, Author