[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH] printf: fix format string sanity check
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2017-07-18 14:02:41
Message-ID: CAK1hOcOGrYCx9jZkR=b14DcdkxVCVXW=9_xQPbWwEquuqDzuwA () mail ! gmail ! com
[Download RAW message or body]

Applied, thanks!

On Tue, Jul 18, 2017 at 10:33 AM, Ron Yorston <rmy@pobox.com> wrote:
> One of the tests for printf checks for an invalid bare '%' in the
> format string:
>
>    $ busybox printf '%' a b c
>    printf: %: invalid format
>
> On x86_64 a slightly different test doesn't work correctly:
>
>    $ busybox printf '%' d e f
>    printf: invalid number 'd'
>    printf: invalid number 'e'
>    printf: invalid number 'f'
>
> On other platforms the test fails randomly depending on how the
> arguments are laid out in memory.
>
> There are two places in the code where strchr is used to determine if
> a character in the format string is valid.  However, strchr also returns
> a valid pointer if the character being searched for is the null terminator
> thus causing the code to incorrectly suppose that a valid character has
> been found.
>
> Add explicit checks for the null terminator.
>
> Signed-off-by: Ron Yorston <rmy@pobox.com>
> ---
>  coreutils/printf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/coreutils/printf.c b/coreutils/printf.c
> index bc22e0ee7..65bb5a935 100644
> --- a/coreutils/printf.c
> +++ b/coreutils/printf.c
> @@ -305,7 +305,7 @@ static char **print_formatted(char *f, char **argv, int *conv_err)
>                                 }
>                                 break;
>                         }
> -                       if (strchr("-+ #", *f)) {
> +                       if (*f && strchr("-+ #", *f)) {
>                                 ++f;
>                                 ++direc_length;
>                         }
> @@ -348,7 +348,7 @@ static char **print_formatted(char *f, char **argv, int *conv_err)
>                                 static const char format_chars[] ALIGN1 = "diouxXfeEgGcs";
>                                 char *p = strchr(format_chars, *f);
>                                 /* needed - try "printf %" without it */
> -                               if (p == NULL) {
> +                               if (p == NULL || *f == '\0') {
>                                         bb_error_msg("%s: invalid format", direc_start);
>                                         /* causes main() to exit with error */
>                                         return saved_argv - 1;
> --
> 2.13.3
>
> _______________________________________________
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]