Bug 2170 - Potential integer overflow
Summary: Potential integer overflow
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: -current
Hardware: Other Linux
: P5 enhancement
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks: V_6_5
  Show dependency tree / graph
 
Reported: 2013-11-12 15:08 AEDT by Loganaden Velvindron
Modified: 2016-08-02 10:42 AEST (History)
1 user (show)

See Also:

Attachments
potential_overflow_fix (638 bytes, text/plain)
2013-11-12 15:08 AEDT, Loganaden Velvindron 		dtucker: ok+
Details
improve diff (switch 1000 to ULL) (738 bytes, text/plain)
2013-12-02 07:18 AEDT, Loganaden Velvindron 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Loganaden Velvindron 2013-11-12 15:08:46 AEDT 
Created attachment 2373 [details]
potential_overflow_fix

in serverloop.c:

	max_time_milliseconds = options.client_alive_interval * 1000;

client_alive_interval is declare as int.

        int     client_alive_interval;  /*
                                         * poke the client this often to
                                         * see if it's still there

max_time_milliseconds is declared as u_int64_t.

Can this potentially result in an overflow due to multiplication ?
Comment 1 Darren Tucker 2013-11-13 06:57:18 AEDT 
looks reasonable, add to the list for 6.5
Comment 2 Loganaden Velvindron 2013-12-02 07:18:50 AEDT 
Created attachment 2379 [details]
improve diff (switch 1000 to ULL)

switch 1000 from integer to unsigned long long.
Comment 3 Loganaden Velvindron 2013-12-09 00:18:36 AEDT 
ping :-) ?

Any feedback on the 1 line diff :-) ?
Comment 4 Darren Tucker 2013-12-19 11:19:58 AEDT 
Damien and I ended up having a bikeshed discussion on this :-)

I think casting the second argument is unnecessary since it'll get promoted to unsigned long long anyway.

First patch applied, it'll be in the next release.

Thanks.
Comment 5 Loganaden Velvindron 2013-12-19 15:26:04 AEDT 
(In reply to Darren Tucker from comment #4)
> Damien and I ended up having a bikeshed discussion on this :-)
> 
> I think casting the second argument is unnecessary since it'll get
> promoted to unsigned long long anyway.
> 
> First patch applied, it'll be in the next release.
> 
> Thanks.

Thank you very much !
Comment 6 Damien Miller 2016-08-02 10:42:03 AEST 
Close all resolved bugs after 7.3p1 release