Bug 3250 - Integer overflow in ConnectTimeout
Summary: Integer overflow in ConnectTimeout
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 8.4p1
Hardware: Other Linux
: P5 minor
Assignee: Assigned to nobody
URL:
Keywords:
: 3256 (view as bug list)
Depends on:
Blocks: V_8_5
  Show dependency treegraph
 
Reported: 2021-01-10 20:04 AEDT by Davide Berardi
Modified: 2023-01-13 13:27 AEDT (History)
4 users (show)

See Also:


Attachments
Proposed Patch (298 bytes, patch)
2021-01-10 20:04 AEDT, Davide Berardi
no flags Details | Diff
convtime return long -> int (3.61 KB, patch)
2021-01-11 11:37 AEDT, Darren Tucker
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Davide Berardi 2021-01-10 20:04:05 AEDT
Created attachment 3465 [details]
Proposed Patch

Setting a high value for ConnectionTimeout option will make it
negative.  This will result in an integer overflow undetected by the
previous checks.

PoC: (ArchLinux GNU/Linux)
$ uname -a
Linux haigha 5.10.5-arch1-1 #1 SMP PREEMPT Thu, 07 Jan 2021 09:50:43 +0000 x86_64 GNU/Linux

$ gcc --version
gcc (GCC) 10.2.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ clang --version
clang version 11.0.0
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
$ ./ssh -V
OpenSSH_8.4p1, OpenSSL 1.1.1i  8 Dec 2020
$ ./ssh -o ConnectTimeout=$(( 0x80000000 )) localhost
[1]    26360 abort (core dumped)  ./ssh -o ConnectTimeout=$(( 0x80000000 )) localhost
Comment 1 Darren Tucker 2021-01-11 11:37:20 AEDT
Created attachment 3466 [details]
convtime return long -> int

THere a bit more to it than that.  convtime() returns -1 on error including negative values, but it returns a long.  On a platform where sizeof(int) != sizeof(long), convtime can accept a large positive value that then wraps to negative.  I think the correct thing to do is to change convtime to return int.
Comment 2 Darren Tucker 2021-01-12 07:36:14 AEDT
patch has been applied and will be in 8.5 release.  thanks for the report.

$ ./ssh -o ConnectTimeout=$(( 0x80000000 )) localhost
command-line line 0: invalid time value.
Comment 3 Damien Miller 2021-01-25 10:44:12 AEDT
*** Bug 3256 has been marked as a duplicate of this bug. ***
Comment 4 Damien Miller 2021-03-04 09:53:59 AEDT
close bugs that were resolved in OpenSSH 8.5 release cycle