[PATCH v2] os: fix pnprintf OOB buffer read for unterminated length modifiers

[walter harms]

Am 14.02.2013 07:31, schrieb Peter Hutterer:
> Format strings with length modifiers but missing format specifier like "%0"
> will one past the array size.
> 
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> sorry, had a messy tree here and the test got affected. This one will
> actually trigger the condition, the \n at the end in the first patch will be
> interpreted as directive otherwise.
> 
>  os/log.c              | 3 +++
>  test/signal-logging.c | 8 ++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/os/log.c b/os/log.c
> index 2697ace..95bd8cc 100644
> --- a/os/log.c
> +++ b/os/log.c
> @@ -304,6 +304,9 @@ pnprintf(char *string, size_t size, const char *f, va_list args)
>          while (f_idx < f_len && ((f[f_idx] >= '0' && f[f_idx] <= '9') || f[f_idx] == '.'))
>              f_idx++;
>  
> +        if (f_idx >= f_len)
> +            break;
> +

why is a private snprintf needed in the first place ?

just my 2 cents,
re,
 wh

>          switch (f[f_idx]) {
>          case 's':
>              string_arg = va_arg(args, char*);
> diff --git a/test/signal-logging.c b/test/signal-logging.c
> index 1ef17af..e0eb810 100644
> --- a/test/signal-logging.c
> +++ b/test/signal-logging.c
> @@ -199,6 +199,14 @@ static void logging_format(void)
>      read_log_msg(logmsg);
>      assert(strcmp(logmsg, "(EE) substituted string\n") == 0);
>  
> +    /* Invalid format */
> +#warning Ignore compiler warning below "lacks type at end of format".  This is intentional.
> +    LogMessageVerbSigSafe(X_ERROR, -1, "%4", 4);
> +    read_log_msg(logmsg);
> +    assert(strcmp(logmsg, "(EE) ") == 0);
> +    LogMessageVerbSigSafe(X_ERROR, -1, "\n");
> +    fseek(f, 0, SEEK_END);
> +
>      /* number substitution */
>      ui = 0;
>      do {