Fwd: New Defects reported by Coverity Scan for zsh

Fwd: New Defects reported by Coverity Scan for zsh

[Bart Schaefer]

On Mon, Oct 16, 2023 at 7:15 PM <scan-admin@coverity.com> wrote:
>
> >>>     CID 1547833:  Memory - illegal accesses  (NEGATIVE_RETURNS)
> >>>     Using variable "*lineptr" as an index to array "typtab".
> /Src/hist.c: 3809 in histsplitwords()

Needs cast to (unsigned char) after removal of STOUC() macro.

> >>>     CID 1547832:  Resource leaks  (RESOURCE_LEAK)
> >>>     Variable "buf" going out of scope leaks the storage it points to.
> /Src/input.c: 668 in stuff()

Pretty sure this is spurious, because zstuff() returns -1 only when it
either hasn't allocated or has already freed the storage pointed to by
buf.

> ** CID 1547831:    (UNUSED_VALUE)
> /Src/Zle/compresult.c: 2090 in printlist()
> /Src/Zle/compresult.c: 1997 in printlist()

These appear to be accurate, pnl is always immediately assigned 1
after being assigned 0.  Also these appear to have been here forever,
not sure why it's just flagged now.

> >>>     CID 1547830:  Error handling issues  (CHECKED_RETURN)
> >>>     "fread(void * restrict, size_t, size_t, FILE * restrict)" returns the number of bytes read, but it is ignored.
> /Src/input.c: 632 in zstuff()

This is actually incorrect on Coverity's part -- as already discussed,
fread() returns the number of objects read, not the number of bytes.

> >>>     CID 1547829:  Error handling issues  (NEGATIVE_RETURNS)
> >>>     "len" is passed to a parameter that cannot be negative.
> /Src/input.c: 632 in zstuff()

Is it really necessary to check whether ftell(in) returned error after
fseek(in, 0, SEEK_END) ?

> >>>     CID 1547828:  Memory - illegal accesses  (NEGATIVE_RETURNS)
> >>>     Using variable "*s" as an index to array "typtab".
> /Src/subst.c: 2559 in paramsubst()

More unsigned casting.  Maybe inblank() and inull() should just do
always cast their argument?  There may actually be 2 more of these not
yet detected.

> *** CID 1547827:  Null pointer dereferences  (FORWARD_NULL)
> /Src/Modules/pcre.c: 370 in bin_pcre_match()
> >>>     Passing null pointer "named" to "zpcre_get_substrings", which dereferences it.

This is from Oliver's 51738 (PCRE's alternative DFA), I'm not going to
interpret futher.

> ** CID 1547826:  Resource leaks  (RESOURCE_LEAK)
> /Src/input.c: 655 in ztuff()

I guess this theoretically could leak one byte.  From an unused
subroutine that I didn't bother to delete.

Re: Fwd: New Defects reported by Coverity Scan for zsh

[Oliver Kiddle]

On 18 Oct, Bart Schaefer wrote:
> > *** CID 1547827:  Null pointer dereferences  (FORWARD_NULL)
> > /Src/Modules/pcre.c: 370 in bin_pcre_match()
> > >>>     Passing null pointer "named" to "zpcre_get_substrings", which dereferences it.
>
> This is from Oliver's 51738 (PCRE's alternative DFA), I'm not going to
> interpret futher.

The DFA matching doesn't support captures so I wouldn't expect it to
find any. However it counts them at compile time rather than when
matching so explicitly guarding against this condition probably is
needed.

Oliver

diff --git a/Src/Modules/pcre.c b/Src/Modules/pcre.c
index 96f3c6e65..f5cda6d38 100644
--- a/Src/Modules/pcre.c
+++ b/Src/Modules/pcre.c
@@ -187,7 +187,8 @@ zpcre_get_substrings(pcre2_code *pat, char *arg, pcre2_match_data *mdata,
 	    setaparam(substravar, matches);
 	}
 
-	if (!pcre2_pattern_info(pat, PCRE2_INFO_NAMECOUNT, &ncount) && ncount
+	if (namedassoc
+		&& !pcre2_pattern_info(pat, PCRE2_INFO_NAMECOUNT, &ncount) && ncount
 		&& !pcre2_pattern_info(pat, PCRE2_INFO_NAMEENTRYSIZE, &nsize)
 		&& !pcre2_pattern_info(pat, PCRE2_INFO_NAMETABLE, &ntable))
 	{