Re: [PATCH] ui: fix DIV_BY_ZERO in tightvnc

[Marc-André Lureau]

Hi

On Wed, Dec 13, 2023 at 11:39 AM Дмитрий Фролов <frolov@swemel.ru> wrote:
>
>
>
> On 13.12.2023 10:31, Marc-André Lureau wrote:
> > Hi
> >
> > On Wed, Dec 13, 2023 at 11:08 AM Dmitry Frolov <frolov@swemel.ru> wrote:
> >> Division by zero may occur in rare constellation of conditions if:
> >> 1. not TrueColor mode on the client side
> >>     tight_detect_smooth_image16() and tight_detect_smooth_image32(),
> >>     defined by macro DEFINE_DETECT_FUNCTION()2, are affected.
> >> 2. if all pixels on the screen are equal, then pixels == stats[0]
> >>
> >> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> >>
> >> Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
> > What about the tight_detect_smooth_image24() division?
> >      errors /= (pixels * 3 - stats[0]);
> Here everything is OK, because there is a check some lines above:
>      if (stats[0] * 33 / pixels >= 95) {
>          return 0;
>      }
> thus, stats[0] < pixels*95/33,
> 95/33 < 3.
> >


/me shivers.. looks legit, yet so easy to get wrong :)

> > It should probably have a similar safety check.
> >
> > The code is originally from libvncserver, but they completely changed
> > their implementation in:
> > https://github.com/LibVNC/libvncserver/commit/7124b5fbcf0df8db4d3f73023d77af6ea56409e7
> > otherwise,
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> >> ---
> >>   ui/vnc-enc-tight.c | 3 +++
> >>   1 file changed, 3 insertions(+)
> >>
> >> diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
> >> index 41f559eb83..f1249ab136 100644
> >> --- a/ui/vnc-enc-tight.c
> >> +++ b/ui/vnc-enc-tight.c
> >> @@ -284,6 +284,9 @@ tight_detect_smooth_image24(VncState *vs, int w, int h)
> >>           for (; c < 256; c++) {                                          \
> >>               errors += stats[c] * (c * c);                               \
> >>           }                                                               \
> >> +        if (pixels == stats[0]) {                                       \
> >> +            return 0;                                                   \
> >> +        }                                                               \
> >>           errors /= (pixels - stats[0]);                                  \
> >>                                                                           \
> >>           return errors;                                                  \
> >> --
> >> 2.34.1
> >>
>