[PATCH] ui/vnc-clipboard: fix infinite loop in inflate_buffer (CVE-2023-3255)

Mauro Matteo Cascella posted 1 patch 9 months, 4 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230704084210.101822-1-mcascell@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>
ui/vnc-clipboard.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
[PATCH] ui/vnc-clipboard: fix infinite loop in inflate_buffer (CVE-2023-3255)
Posted by Mauro Matteo Cascella 9 months, 4 weeks ago
A wrong exit condition may lead to an infinite loop when inflating a
valid zlib buffer containing some extra bytes in the `inflate_buffer`
function. The bug only occurs post-authentication. Return the buffer
immediately if the end of the compressed data has been reached
(Z_STREAM_END).

Fixes: CVE-2023-3255
Fixes: 0bf41cab ("ui/vnc: clipboard support")
Reported-by: Kevin Denis <kevin.denis@synacktiv.com>
Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
---
 ui/vnc-clipboard.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/ui/vnc-clipboard.c b/ui/vnc-clipboard.c
index 8aeadfaa21..c759be3438 100644
--- a/ui/vnc-clipboard.c
+++ b/ui/vnc-clipboard.c
@@ -50,8 +50,11 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t in_len, uint32_t *size)
         ret = inflate(&stream, Z_FINISH);
         switch (ret) {
         case Z_OK:
-        case Z_STREAM_END:
             break;
+        case Z_STREAM_END:
+            *size = stream.total_out;
+            inflateEnd(&stream);
+            return out;
         case Z_BUF_ERROR:
             out_len <<= 1;
             if (out_len > (1 << 20)) {
@@ -66,11 +69,6 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t in_len, uint32_t *size)
         }
     }
 
-    *size = stream.total_out;
-    inflateEnd(&stream);
-
-    return out;
-
 err_end:
     inflateEnd(&stream);
 err:
-- 
2.41.0
Re: [PATCH] ui/vnc-clipboard: fix infinite loop in inflate_buffer (CVE-2023-3255)
Posted by Marc-André Lureau 9 months, 4 weeks ago
On Tue, Jul 4, 2023 at 10:42 AM Mauro Matteo Cascella <mcascell@redhat.com>
wrote:

> A wrong exit condition may lead to an infinite loop when inflating a
> valid zlib buffer containing some extra bytes in the `inflate_buffer`
> function. The bug only occurs post-authentication. Return the buffer
> immediately if the end of the compressed data has been reached
> (Z_STREAM_END).
>
> Fixes: CVE-2023-3255
> Fixes: 0bf41cab ("ui/vnc: clipboard support")
> Reported-by: Kevin Denis <kevin.denis@synacktiv.com>
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
>

Tested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Note: we may want to disconnect the client when there are extra bytes in
the message, or print some warnings.


> ---
>  ui/vnc-clipboard.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/ui/vnc-clipboard.c b/ui/vnc-clipboard.c
> index 8aeadfaa21..c759be3438 100644
> --- a/ui/vnc-clipboard.c
> +++ b/ui/vnc-clipboard.c
> @@ -50,8 +50,11 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t
> in_len, uint32_t *size)
>          ret = inflate(&stream, Z_FINISH);
>          switch (ret) {
>          case Z_OK:
> -        case Z_STREAM_END:
>              break;
> +        case Z_STREAM_END:
> +            *size = stream.total_out;
> +            inflateEnd(&stream);
> +            return out;
>          case Z_BUF_ERROR:
>              out_len <<= 1;
>              if (out_len > (1 << 20)) {
> @@ -66,11 +69,6 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t
> in_len, uint32_t *size)
>          }
>      }
>
> -    *size = stream.total_out;
> -    inflateEnd(&stream);
> -
> -    return out;
> -
>  err_end:
>      inflateEnd(&stream);
>  err:
> --
> 2.41.0
>
>
>

-- 
Marc-André Lureau
Re: [PATCH] ui/vnc-clipboard: fix infinite loop in inflate_buffer (CVE-2023-3255)
Posted by Mauro Matteo Cascella 9 months, 4 weeks ago
On Tue, Jul 4, 2023 at 11:03 AM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
>
>
> On Tue, Jul 4, 2023 at 10:42 AM Mauro Matteo Cascella <mcascell@redhat.com> wrote:
>>
>> A wrong exit condition may lead to an infinite loop when inflating a
>> valid zlib buffer containing some extra bytes in the `inflate_buffer`
>> function. The bug only occurs post-authentication. Return the buffer
>> immediately if the end of the compressed data has been reached
>> (Z_STREAM_END).
>>
>> Fixes: CVE-2023-3255
>> Fixes: 0bf41cab ("ui/vnc: clipboard support")
>> Reported-by: Kevin Denis <kevin.denis@synacktiv.com>
>> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
>
>
> Tested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Note: we may want to disconnect the client when there are extra bytes in the message, or print some warnings.

Sure, I guess we can call vnc_disconnect_finish or vnc_client_error
for disconnecting, not sure how to properly print warnings. Feel free
to add that yourself when applying the patch. Or I can try to send v2
if you prefer.

Thanks,

>>
>> ---
>>  ui/vnc-clipboard.c | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/ui/vnc-clipboard.c b/ui/vnc-clipboard.c
>> index 8aeadfaa21..c759be3438 100644
>> --- a/ui/vnc-clipboard.c
>> +++ b/ui/vnc-clipboard.c
>> @@ -50,8 +50,11 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t in_len, uint32_t *size)
>>          ret = inflate(&stream, Z_FINISH);
>>          switch (ret) {
>>          case Z_OK:
>> -        case Z_STREAM_END:
>>              break;
>> +        case Z_STREAM_END:
>> +            *size = stream.total_out;
>> +            inflateEnd(&stream);
>> +            return out;
>>          case Z_BUF_ERROR:
>>              out_len <<= 1;
>>              if (out_len > (1 << 20)) {
>> @@ -66,11 +69,6 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t in_len, uint32_t *size)
>>          }
>>      }
>>
>> -    *size = stream.total_out;
>> -    inflateEnd(&stream);
>> -
>> -    return out;
>> -
>>  err_end:
>>      inflateEnd(&stream);
>>  err:
>> --
>> 2.41.0
>>
>>
>
>
> --
> Marc-André Lureau



-- 
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH] ui/vnc-clipboard: fix infinite loop in inflate_buffer (CVE-2023-3255)
Posted by Michael Tokarev 9 months, 2 weeks ago
04.07.2023 12:39, Mauro Matteo Cascella wrote:
> On Tue, Jul 4, 2023 at 11:03 AM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
>>
>>
>>
>> On Tue, Jul 4, 2023 at 10:42 AM Mauro Matteo Cascella <mcascell@redhat.com> wrote:
>>>
>>> A wrong exit condition may lead to an infinite loop when inflating a
>>> valid zlib buffer containing some extra bytes in the `inflate_buffer`
>>> function. The bug only occurs post-authentication. Return the buffer
>>> immediately if the end of the compressed data has been reached
>>> (Z_STREAM_END).
>>>
>>> Fixes: CVE-2023-3255
>>> Fixes: 0bf41cab ("ui/vnc: clipboard support")
>>> Reported-by: Kevin Denis <kevin.denis@synacktiv.com>
>>> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
>>
>>
>> Tested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Note: we may want to disconnect the client when there are extra bytes in the message, or print some warnings.
> 
> Sure, I guess we can call vnc_disconnect_finish or vnc_client_error
> for disconnecting, not sure how to properly print warnings. Feel free
> to add that yourself when applying the patch. Or I can try to send v2
> if you prefer.

Ping, for 8.1 and -stable?

Thanks,

/mjt