Re: [Qemu-devel] [PATCH v1 00/13] Fix VNC server unbounded memory usage

[Darren Kenny]

Hi Daniel,

For the series:

Reviewed-by: Darren Kenny <address@hidden>

With one small nit on patch 1.

Thanks,

Darren.

On Mon, Dec 18, 2017 at 07:12:15PM +0000, Daniel P. Berrange wrote:
> In the 2.11 release we fixed CVE-2017-15268, which allowed the VNC websockets
> server to consume arbitrary memory when a slow client was connected. I have
> since discovered that this same type of problem can be triggered in several
> other ways in the regular (non-websockets) VNC server. This patch series
> attempts to fix this problem by limiting framebuffer updates and other data
> sent from server to client. The mitigating factor is that you need to have
> successfully authenticated with the VNC server to trigger these new flaws.
> This new more general flaw is assigned CVE-2017-15124 by the Red Hat security
> team.
>
> The key patches containing the security fix are 9, 10, 11.
>
> Since this code is incredibly subtle & hard to understand though, the first
> 8 patches do a bunch of independant cleanups/refactoring to make the security
> fixes clearer.  The last two patches are just some extra cleanup / help for
> future maint.
>
> Daniel P. Berrange (13):
>  ui: remove 'sync' parametr from vnc_update_client
>  ui: remove unreachable code in vnc_update_client
>  ui: remove redundant indentation in vnc_client_update
>  ui: avoid pointless VNC updates if framebuffer isn't dirty
>  ui: track how much decoded data we consumed when doing SASL encoding
>  ui: introduce enum to track VNC client framebuffer update request
>    state
>  ui: correctly reset framebuffer update state after processing dirty
>    regions
>  ui: refactor code for determining if an update should be sent to the
>    client
>  ui: fix VNC client throttling when audio capture is active
>  ui: fix VNC client throttling when forced update is requested
>  ui: place a hard cap on VNC server output buffer size
>  ui: add trace events related to VNC client throttling
>  ui: mix misleading comments & return types of VNC I/O helper methods
>
> ui/trace-events    |   7 ++
> ui/vnc-auth-sasl.c |  16 ++-
> ui/vnc-auth-sasl.h |   5 +-
> ui/vnc-jobs.c      |   5 +
> ui/vnc.c           | 320 ++++++++++++++++++++++++++++++++++++++---------------
> ui/vnc.h           |  28 ++++-
> 6 files changed, 277 insertions(+), 104 deletions(-)
>
> --
> 2.14.3