ui/vnc-enc-tight.c | 3 +++ 1 file changed, 3 insertions(+)
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>
---
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.1Hi
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]);
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
>
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.
>
> 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
>>
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
> >>
>
© 2016 - 2025 Red Hat, Inc.