Re: [PATCH] timing.cc integer overflow

[Henri Doreau <henri.doreau () gmail com>]

Thanks for your reply. Here are further elements.

> Can you find a set of values that cause an overflow, to demonstrate that
> the overflow is fixed in at least one case? I mean values for begin,
> now, and perc_done.
    - At 20% of a scan
    - Scan started 10 days ago
    Output :
        Real current time : Mon Jan 26 14:00:43 2009
        Assuming this pseudo scan started at Fri Jan 16 14:00:43 2009
        UDP scan Timing: About 20.00% done; ETC: 20:57 (-233:-2:-47 remaining)

> I like to make sure that a bug is reproducible before it's fixed, so
> that the fix can be shown to work in at least one case. If you can, find
> values that cause an overflow and send them in.
The bug is reproducible, I've triggered it for several values of
involved variables.
Here are very detailed explanations and an array of time that scans
have to last to overflow unsigned long (for several values of
perc_done ).
50days is a limit because ULONG_MAX == 4294967295 (at least for
platforms where long ints are stored on 4bytes)
so an unsigned long can contain up to the equivalent of 49.7 days in
milliseconds.

The integer overflow we can see come from one of these two lines,
according to the case

(1)    time_used_ms = TIMEVAL_MSEC_SUBTRACT(*now, begin); // Overflow
if the scan is running from more than 50 days
       [...]
(2)    time_needed_ms = (unsigned long) ((double) time_used_ms /
perc_done); // Can overflow too



+-----------+------------------------------------+------+
| perc_done |    Overflow if time elapsed > ..   | line |
+-----------+------------------------------------+------+
|           |                                    |      |
|    11%    |   7 days                           |  2   |
|           |                                    |      |
+-----------+------------------------------------+------+
|           |                                    |      |
|    20%    |   13 days                          |  2   |
|           |                                    |      |
+-----------+------------------------------------+------+
|           |                                    |      |
|    30%    |   22 days                          |  2   |
|           |                                    |      |
+-----------+------------------------------------+------+
|           |                                    |      |
|    40%    |   34 days                          |  2   |
|           |                                    |      |
+-----------+------------------------------------+------+
|           |                                    |      |
|    50%    |   50 days                          |  1   |
|           |                                    |      |
+-----------+------------------------------------+------+
|           |                                    |      |
|    60%    |   50 days                          |  1   |
|           |                                    |      |
+-----------+------------------------------------+------+
|           |                                    |      |
|    70%    |   50 days                          |  1   |
|           |                                    |      |
+-----------+------------------------------------+------+
|           |                                    |      |
|    80%    |   50 days                          |  1   |
|           |                                    |      |
+-----------+------------------------------------+------+
|           |                                    |      |
|    90%    |   50 days                          |  1   |
|           |                                    |      |
+-----------+------------------------------------+------+

So using unsigned long doesn't appear as a reliable solution, I tried
using doubles and print days if current scan is expected to last more
than N days (I've arbitrarily choosen 5).
If I made no mistake, my tests show that at 11% done, the overflow
only occurs if the scan was launched 6145 days ago, this will let
people some time to "UDP-scan" the whole internet (again)!

I hope it is good now.

Cheers


Henri

Attachment: timing.h.patch
Description:

Attachment: timing.cc.patch
Description:

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org