[syslog-ng] Seg fault with include file contains an error

[Balazs Scheidler]

Hi,

The patch looks good and I could successfully reproduce the problem
without the patch and it seems to be gone with the patch.

Thanks for tracking this down, and sorry that it took such a long time
to review and integrate your patch.

Here's the official git patch entry, now visible on git.balabit.hu

commit 45566d957609026abe41a3292003909b23615124
Author: Anthony Lineham <anthony.lineham at alliedtelesis.co.nz>
Date:   Tue Nov 3 20:44:59 2009 +0100

    cfg-lex: fixed a possible segmentation fault in HUP processing if
included files have syntax errors
    
    This patch fixes several use-after-free problems if a file with a
    syntax error is included and syslog-ng is reloaded multiple times.
    
    Thanks for Anthony Lineham for tracking this down.



On Mon, 2009-09-14 at 16:50 +1200, anthony lineham wrote:
> Hi Bazsi,
> 
> I've been investigating a seg fault that occurs when using an include statement in the config file. The error 
> occurs when one of the included files contains an syntax error. The first time syslog-ng is restarted with the bad
> config, the error is detected and restart is aborted. However, if a second HUP is sent a seg fault occurs.
> 
> I had a bit of trouble tracking down the cause and I'm still not entirely sure of the mechanism that leads to the
> crash but I found a couple of apparent problems, which when corrected prevent it.
> 
> 1. There is a global variable "include_depth" which normally gets decremented back to zero after successful 
> parsing of included config files. However, if an error is detected it stays at its current value and subsequent 
> restarts increment from that point. This doesn't cause the crash, but given enough restarts would lead to
> overrun or exhaustion of the "include_stack" array.
> 
> 2. In the case of unsuccessful config parsing certain bits of memory are dealloced but their pointers that not
> reset. This seems to be what was causing the seg fault but I've found it a bit hard to pin down.
> 
> 3. There was an off-by-1 error in the included config deinit loop.
> 
> The following patch fixes the problem, but may not necessarily be the best way to do it - particularly issue 1.
> 
> Regards,
> Anthony
> 
> --- syslog-ng_3.0.1-63-g41f77f5-old/src/cfg-lex.l       2009-09-14 16:02:09.000000000 +1200
> +++ syslog-ng_3.0.1-63-g41f77f5/src/cfg-lex.l   2009-09-14 16:15:30.000000000 +1200        
> @@ -626,17 +626,24 @@
>  {
>    gint i;
> 
> -  for (i = 0; i < include_depth; i++)
> +  for (i = 0; i <= include_depth; i++)
>      {
>        CfgIncludeLevel *level = &include_stack[i];
> 
>        if (level->current_file)
> -        g_free(level->current_file);
> +        {
> +          g_free(level->current_file);
> +          level->current_file = NULL;
> +        }
> 
>        g_slist_foreach(level->files, (GFunc) g_free, NULL);
>        g_slist_free(level->files);
>        level->files = NULL;
>        if (level->yybuf)
> -        yy_delete_buffer(level->yybuf);
> +        {
> +          yy_delete_buffer(level->yybuf);
> +          level->yybuf = NULL;
> +        }
>      }
> +  include_depth = 0;
>  }
> 
> 
> ______________________________________________________________________________
> Member info: https://lists.balabit.hu/mailman/listinfo/syslog-ng
> Documentation: http://www.balabit.com/support/documentation/?product=syslog-ng
> FAQ: http://www.campin.net/syslog-ng/faq.html
> 
> 
-- 
Bazsi