infinite recursion in ihungetc()

infinite recursion in ihungetc()

[Kamil Dudka]

Fedora Analysis Framework captured 10 crashes of zsh-5.0.8 due to infinite 
recursion in ihungetc():

https://retrace.fedoraproject.org/faf/reports/717794/

The infinite recursion happens at this line:

http://repo.or.cz/w/zsh/mirror.git/blob/a0862f63:/Src/hist.c#l908

The 'hungetc' code pointer is obviously set to ihungetc() itself.  We do
not have full bakctrace of the crash but shouldn't there be any condition
to actually stop the recursion when the conditions at lines 906-907 are
true and hungetc points at ihungetc()?

There is no single command that could invalidate any of the conditions
after nesting deeper into the recursion...

Kamil

Re: infinite recursion in ihungetc()

[Peter Stephenson]

On Wed, 22 Jul 2015 13:45:06 +0200
Kamil Dudka <kdudka@redhat.com> wrote:
> Fedora Analysis Framework captured 10 crashes of zsh-5.0.8 due to infinite 
> recursion in ihungetc():
> 
> https://retrace.fedoraproject.org/faf/reports/717794/
> 
> The infinite recursion happens at this line:
> 
> http://repo.or.cz/w/zsh/mirror.git/blob/a0862f63:/Src/hist.c#l908
> 
> The 'hungetc' code pointer is obviously set to ihungetc() itself.  We do
> not have full bakctrace of the crash but shouldn't there be any condition
> to actually stop the recursion when the conditions at lines 906-907 are
> true and hungetc points at ihungetc()?
> 
> There is no single command that could invalidate any of the conditions
> after nesting deeper into the recursion...

Yes, it should be straightforward to prevent, although this code isn't
particularly transparent (or new), so I don't know how you could get
there.

pws

diff --git a/Src/hist.c b/Src/hist.c
index 6725313..cf224cb 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -136,6 +136,7 @@ mod_export int hist_skip_flags;
 #define HA_NOINC	(1<<1)	/* Don't store, curhist not incremented */
 #define HA_INWORD       (1<<2)  /* We're inside a word, don't add
 				   start and end markers */
+#define HA_UNGET        (1<<3)  /* Recursively ungetting */
 
 /* Array of word beginnings and endings in current history line. */
 
@@ -904,8 +905,13 @@ ihungetc(int c)
 
     while (!lexstop && !errflag) {
 	if (hptr[-1] != (char) c && stophist < 4 &&
-	    hptr > chline + 1 && hptr[-1] == '\n' && hptr[-2] == '\\')
-	    hungetc('\n'), hungetc('\\');
+	    hptr > chline + 1 && hptr[-1] == '\n' && hptr[-2] == '\\' &&
+	    !(histactive & HA_UNGET)) {
+	    histactive |= HA_UNGET;
+	    hungetc('\n');
+	    hungetc('\\');
+	    histactive &= ~HA_UNGET;
+	}
 
 	if (expanding) {
 	    zlemetacs--;

Re: infinite recursion in ihungetc()

[Kamil Dudka]

On Wednesday 22 July 2015 14:38:03 Peter Stephenson wrote:
> Yes, it should be straightforward to prevent, although this code isn't
> particularly transparent (or new), so I don't know how you could get
> there.
> 
> pws

I do not know it either because these crash reports are anonymous.  I will 
just apply your patch to fix it and let you know if we get any feedback.  
Thanks for the patch!

Kamil

> diff --git a/Src/hist.c b/Src/hist.c
> index 6725313..cf224cb 100644
> --- a/Src/hist.c
> +++ b/Src/hist.c
> @@ -136,6 +136,7 @@ mod_export int hist_skip_flags;
>  #define HA_NOINC	(1<<1)	/* Don't store, curhist not incremented */
>  #define HA_INWORD       (1<<2)  /* We're inside a word, don't add
>  				   start and end markers */
> +#define HA_UNGET        (1<<3)  /* Recursively ungetting */
> 
>  /* Array of word beginnings and endings in current history line. */
> 
> @@ -904,8 +905,13 @@ ihungetc(int c)
> 
>      while (!lexstop && !errflag) {
>  	if (hptr[-1] != (char) c && stophist < 4 &&
> -	    hptr > chline + 1 && hptr[-1] == '\n' && hptr[-2] == '\\')
> -	    hungetc('\n'), hungetc('\\');
> +	    hptr > chline + 1 && hptr[-1] == '\n' && hptr[-2] == '\\' &&
> +	    !(histactive & HA_UNGET)) {
> +	    histactive |= HA_UNGET;
> +	    hungetc('\n');
> +	    hungetc('\\');
> +	    histactive &= ~HA_UNGET;
> +	}
> 
>  	if (expanding) {
>  	    zlemetacs--;