openssh bugs - Sep 2024 - [Bug 3730] New: Integer overflow when adding 6000 and display_number

https://bugzilla.mindrot.org/show_bug.cgi?id=3730

            Bug ID: 3730
           Summary: Integer overflow when adding 6000 and display_number
           Product: Portable OpenSSH
           Version: 9.8p1
          Hardware: Other
                OS: All
            Status: NEW
          Severity: minor
          Priority: P5
         Component: Build system
          Assignee: unassigned-bugs at mindrot.org
          Reporter: suhov.ra at npc-ksb.ru

File:
https://github.com/openssh/openssh-portable/blob/master/channels.c
Line: 5239

There is virtually no chance of integer overflow when adding 6000 to
display_number. The display_number variable is read from a string
containing the value of the DISPLAY environment variable, which
typically points to an X server on the system. The variable is of type
unsigned int, which limits its range of values ??to 0 to UINT_MAX. In
theory, for overflow to occur, display_number must be large enough that
the sum exceeds the maximum value for unsigned int.

The following check can be added to rule out the possibility of integer
overflow:

if (display_number > UINT_MAX - 6000) {
error("display_number too large, possible integer overflow.");
return -1;
}

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3730

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org

--- Comment #1 from Damien Miller <djm at mindrot.org>
---
> The display_number variable is read from a string containing the
> value of the DISPLAY environment variable
That is not correct. This code is used to *set* the DISPLAY environment
variable. The starting value comes from the sshd_config
X11DisplayOffset configuration option.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3730

--- Comment #2 from Damien Miller <djm at mindrot.org> ---
*** Bug 3731 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3730

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at dtucker.net

--- Comment #3 from Darren Tucker <dtucker at dtucker.net> ---
(In reply to Damien Miller from comment #1)
> > The display_number variable is read from a string containing the
> > value of the DISPLAY environment variable
> 
> That is not correct. This code is used to *set* the DISPLAY
> environment variable. The starting value comes from the sshd_config
> X11DisplayOffset configuration option.
There's both.

x11_create_display_inet() uses X11DisplayOffset as a starting point to
figure out what port sshd(8) should try to bind to, after which it sets
$DISPLAY on the server side to point to that.  This is constrained by
MAX_DISPLAYS (1000), but it's got to fit inside a 16-bit port number. 
So in theory the admin could set X11DisplayOffset higher than the
16-bit port number.

x11_connect_display() does parse $DISPLAY to figure out where ssh(1)
should send the forwarded connections to.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3730

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
             Blocks|                            |3740
         Resolution|---                         |FIXED

--- Comment #4 from Darren Tucker <dtucker at dtucker.net> ---
This should be fixed by
https://github.com/openssh/openssh-portable/commit/9998c93d57bf0f1df2bc93e0bc2d8112c6f8c720.
 Thanks for the report.


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=3740
[Bug 3740] Tracking bug for OpenSSH 10.0
-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.