Re: report security problem of nbd

[Manfred Spraul <manfred@colorfullife.com>]

Hi Wangduo,

On 1/24/22 05:10, 王多 wrote:
> 1.stack overflow
> In nbd-server.c, function handle_info have a stack overflow
>
> https://github.com/NetworkBlockDevice/nbd/blob/5750003711b8050bad3ddaf5196201ef419ce15d/nbd-server.c#L2299
>
> len can be controlled by an attacker, the buf size is 1024, when `len 
> - sizeof(namelen) > 1024` the buf overflow.
[...]

Would something like this be a suitable fix?

<<<

> --- a/nbd-server.c
> +++ b/nbd-server.c
> @@ -2356,6 +2356,12 @@static bool handle_info(CLIENT* client, uint32_t 
> opt, GArray* servers, uint32_t
>
>        socket_read(client, &len, sizeof(len));
>        len = htonl(len);
> +if (len > sizeof(buf)) {
> +consume(client, len, buf, sizeof(buf));
> +
> +send_reply(client, opt, NBD_REP_ERR_POLICY, -1, "Access denied by 
> server configuration");
> +return false;
> +}
>        socket_read(client, &namelen, sizeof(namelen));
>        namelen = htonl(namelen);
>        if(namelen > (len - 6)) {
(untested, and obviously the error message is wrong)

>>>

> 2.heap overflow
> In nbd-server.c, function handle_info and handle_export_name have a 
> heap overflow
>
> https://github.com/NetworkBlockDevice/nbd/blob/5750003711b8050bad3ddaf5196201ef419ce15d/nbd-server.c#L2302
> https://github.com/NetworkBlockDevice/nbd/blob/5750003711b8050bad3ddaf5196201ef419ce15d/nbd-server.c#L2117
>
> namelen can be controlled by an attacker, when `namelen = -1`,  malloc 
> will allocate a very small buffer, but socket_read will read a 
> 0xffffffff, thus causing a heap overflow
The pattern is identical: Should we consume the user sent data, and then 
fail?

Or should we perform a hard disconnect the connection if namelen is e.g. 
> INT_MAX?

--

    Manfred