On Fri, Feb 08, 2019 at 01:27:37PM +0000, Pallantla Poornima wrote:
> sprintf function is not secure as it doesn't check the length of string.
> More secure function snprintf is used.
>
> Fixes: 727909c592 ("app/test: introduce dynamic commands list")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Pallantla Poornima <pallantlax.poorn...@intel.com>
> ---
> test/test/commands.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/test/test/commands.c b/test/test/commands.c
> index 94fbc310e..5aeb35498 100644
> --- a/test/test/commands.c
> +++ b/test/test/commands.c
> @@ -367,6 +367,8 @@ int commands_init(void)
> struct test_command *t;
> char *commands, *ptr;
> int commands_len = 0;
> + int total_written = 0;
> + int count = 0;
>
> TAILQ_FOREACH(t, &commands_list, next) {
> commands_len += strlen(t->command) + 1;
> @@ -378,7 +380,10 @@ int commands_init(void)
>
> ptr = commands;
> TAILQ_FOREACH(t, &commands_list, next) {
> - ptr += sprintf(ptr, "%s#", t->command);
> + count = snprintf(ptr, commands_len - total_written - 1, "%s#",
> + t->command);
> + ptr += count;
> + total_written += count;
> }
I don't think the "-1" should be necessary here. Also, I think you should
check the return value of snprintf to check for truncation, and abort the
loop if so.
/Bruce
