Re: [Xen-devel] [PATCH] tools: libxenstat: fix format string overflow

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/02/18 17:36, Dario Faggioli wrote:
> With gcc 7.3.0, the build fails like this:
>
> src/xenstat_linux.c: In function ‘getBridge’
> src/xenstat_linux.c:78:34: warning: ‘%s’ directive writing up to 255 bytes 
> into a region of size 241 [-Wformat-overflow=]
>      sprintf(tmp, "/sys/class/net/%s/bridge", de->d_name);
>                                   ^~
> src/xenstat_linux.c:78:5: note: ‘sprintf’ output between 23 and 278 bytes 
> into a destination of size 256
>      sprintf(tmp, "/sys/class/net/%s/bridge", de->d_name);
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fix using asprintf().
>
> Signed-off-by: Dario Faggioli <dfaggioli@xxxxxxxx>
> ---
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> In case no one else have noticed and fixed this (I have checked xen-devel and
> found nothing)
> ---
>  tools/xenstat/libxenstat/src/xenstat_linux.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/xenstat/libxenstat/src/xenstat_linux.c 
> b/tools/xenstat/libxenstat/src/xenstat_linux.c
> index 907d65fa63..396357511b 100644
> --- a/tools/xenstat/libxenstat/src/xenstat_linux.c
> +++ b/tools/xenstat/libxenstat/src/xenstat_linux.c
> @@ -69,18 +69,20 @@ void getBridge(char *excludeName, char *result, size_t 
> resultLen)
>       struct dirent *de;
>       DIR *d;
>  
> -     char tmp[256] = { 0 };
> -
>       d = opendir("/sys/class/net");
>       while ((de = readdir(d)) != NULL) {
>               if ((strlen(de->d_name) > 0) && (de->d_name[0] != '.')
>                       && (strstr(de->d_name, excludeName) == NULL)) {
> -                             sprintf(tmp, "/sys/class/net/%s/bridge", 
> de->d_name);
> +                             char *tmp;
> +
> +                             asprintf(&tmp, "/sys/class/net/%s/bridge", 
> de->d_name);
>  
>                               if (access(tmp, F_OK) == 0) {

Possible NULL dereference.

IIRC, you need to set tmp to NULL first, otherwise you may free a
spurious pointer on failure, and check for asprintf() returning < 0.

~Andrew

>                                       strncpy(result, de->d_name, resultLen - 
> 1);
>                                       result[resultLen - 1] = 0;
>                               }
> +
> +                             free(tmp);
>               }
>       }
>  
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel