Re: [PATCH] Fix undefined behaviour in test_buffer_printf

[Tim Rühsen]

Hi Andreas,

thanks for your patches :-)

I'll run them through our CI (see
https://gitlab.com/gnuwget/wget2/-/merge_requests/469) now and review
them later this evening.

Regarding the UB: what tool did you use to detect it or was it a manual
review ? As our continuous fuzzer (libFuzzer/afl on OSS-FUZZ) doesn't
detect it...

Regards, Tim

On 11.04.20 22:05, Andreas Schwab wrote:
> * unit-tests/test.c (test_buffer_printf): Cast argument to printf
> function to correct type.
> ---
>  unit-tests/test.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/unit-tests/test.c b/unit-tests/test.c
> index f19b41de..5c1209c6 100644
> --- a/unit-tests/test.c
> +++ b/unit-tests/test.c
> @@ -314,6 +314,8 @@ static void test_buffer_printf(void)
>       static const char *left_adjust[] = { "", "-" };
>       static const long long number[] = { 0, 1LL, -1LL, 10LL, -10LL, 
> 18446744073709551615ULL };
>       static const char *modifier[] = { "", "h", "hh", "l", "ll", "z" }; // 
> %L... won't work on OpenBSD5.0
> +     enum argtype { type_int, type_long, type_long_long, type_size_t };
> +     static const enum argtype modifier_type[] = { type_int, type_int, 
> type_int, type_long, type_long_long, type_size_t };
>       static const char *conversion[] = { "d", "i", "u", "o", "x", "X" };
>       char fmt[32], result[64], string[32];
>       size_t z, a, it, n, c, m;
> @@ -464,8 +466,26 @@ integer_tests:
>                                                       #pragma GCC diagnostic 
> push
>                                                       #pragma GCC diagnostic 
> ignored "-Wformat-nonliteral"
>  #endif
> -                                                     snprintf(result, 
> sizeof(result), fmt, number[n]);
> -                                                     
> wget_buffer_printf(&buf, fmt, number[n]);
> +                                                     switch 
> (modifier_type[m]) {
> +                                                     case type_int:
> +                                                             
> snprintf(result, sizeof(result), fmt, (int)number[n]);
> +                                                             
> wget_buffer_printf(&buf, fmt, (int)number[n]);
> +                                                             break;
> +                                                     case type_long:
> +                                                             
> snprintf(result, sizeof(result), fmt, (long)number[n]);
> +                                                             
> wget_buffer_printf(&buf, fmt, (long)number[n]);
> +                                                             break;
> +                                                     case type_long_long:
> +                                                             
> snprintf(result, sizeof(result), fmt, (long long)number[n]);
> +                                                             
> wget_buffer_printf(&buf, fmt, (long long)number[n]);
> +                                                             break;
> +                                                     case type_size_t:
> +                                                             
> snprintf(result, sizeof(result), fmt, (size_t)number[n]);
> +                                                             
> wget_buffer_printf(&buf, fmt, (size_t)number[n]);
> +                                                             break;
> +                                                     default:
> +                                                             abort();
> +                                                     }
>  #if defined __clang__ || __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 
> 5)
>                                                       #pragma GCC diagnostic 
> pop
>  #endif
>

signature.asc
Description: OpenPGP digital signature