[pve-devel] [PATCH] fix read after string end

Thomas Lamprecht t.lamprecht at proxmox.com
Tue Jun 18 14:37:54 CEST 2019


On 6/18/19 2:24 PM, Dominik Csapak wrote:
> outs is not a zero-terminated string but has its length given by
> outslen, so use that (with a maximum of the size of msg)
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  RADOS.xs | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/RADOS.xs b/RADOS.xs
> index f3f5516..7eca024 100644
> --- a/RADOS.xs
> +++ b/RADOS.xs
> @@ -131,8 +131,14 @@ CODE:
>  
>      if (ret < 0) {
>          char msg[4096];
> -        snprintf(msg, sizeof(msg), "mon_command failed - %s\n", outs);
> +        if (outslen > sizeof(msg)) {
> +            outslen = sizeof(msg);
> +        }

we always have some static string part which already consumes from
msg available space, so above is wrong? Why not just omit it, snprintf
will never print more than sizeof(msg) anyway?

> +        snprintf(msg, sizeof(msg), "mon_command failed - %.*s\n", (int)outslen, outs);

so, .* is using the field width functionallity of the *printf methods,
man snprintfs tells me:

> Field width
> ... In no case does a nonexistent or small field width cause truncation of a field; if the result of  a  conversion  is
> wider than the field width, the field is expanded to contain the conversion result.

So is this actually working?

>          rados_buffer_free(outs);
> +        if (outbuf != NULL) {
> +            rados_buffer_free(outbuf);
> +        }
>          die(msg);
>      }
>  
> 





More information about the pve-devel mailing list