Re: [PATCH] efi: fix out-of-bounds null overwrite vulnerability

From: Matt Fleming
Date: Mon Jan 11 2016 - 09:16:18 EST


On Fri, 08 Jan, at 04:47:17PM, Luck, Tony wrote:
> > But this function doesn't use snprintf(), it uses scnprintf() which
> > returns the number of characters written into buf and, because
> > scnprintf() largely follows vnsprintf(), it will never write more than
> > 'size' bytes into the buffer.
>
> if (bank && device)
> n = snprintf(msg, len, "DIMM location: %s %s ", bank, device);
>
> That looks like "snprintf", not "scnprintf" to me :-)

Oops! Can you believe I looked at the wrong function?

> What about using:
>
> msg[len] = '\0';
>
> to guarantee NUL termination?

But that may leave garbage bytes in 'rcd_decode_str' in the case where
the string isn't as long as 'len'.

How about memset()'ing the buffer to zero and deleting the NUL
termination line?