openssh bugs - May 2020 - [Bug 3171] New: Error in time conversion

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

            Bug ID: 3171
           Summary: Error in time conversion
           Product: Portable OpenSSH
           Version: -current
          Hardware: All
                OS: All
            Status: NEW
          Severity: minor
          Priority: P5
         Component: Miscellaneous
          Assignee: unassigned-bugs at mindrot.org
          Reporter: ronf at timeheart.net

While experimenting with the RekeyLimit option, I ran across a small
bug in the convtime() function. When I entered a time value of '1m30s',
I found that it converted this to 1860 seconds instead of the expected
90 seconds. Entering it as '30s1m' worked fine as a workaround (as
would just entering "90"), but clearly the conversion is incorrect.

The bug appears to be that the convtime() function doesn't reset the
multiplier back to 1 when it sees the 's'. So, whatever the previous
multiplier was (60 in my example) is applied to the number before the
's'. This only works right when a seconds back is specified alone, or
first.

Here's the code in question:

                switch (*endp++) {
                case '\0':
                        endp--;
                        break;
                case 's':
                case 'S':
                        break;
                case 'm':
                case 'M':
                        multiplier = MINUTES;
                        break;
                case 'h':
                case 'H':
                        multiplier = HOURS;
                        break;
                case 'd':
                case 'D':
                        multiplier = DAYS;
                        break;
                case 'w':
                case 'W':
                        multiplier = WEEKS;
                        break;
                default:
                        return -1;
                }

One option would be to have a "multiplier = 1" before the
"break" for
's'/'S'. There's also a problem with '1m30', though.
So, perhaps the
better fix is to set the multiplier back to 1 inside the while loop,
perhaps right before the switch.

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

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

Darren Tucker <dtucker at dtucker.net> changed:

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


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=3162
[Bug 3162] Tracking bug for 8.4 release
-- 
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=3171

--- Comment #1 from Darren Tucker <dtucker at dtucker.net> ---
Created attachment 3400
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3400&action=edit
fix convtime, add unit test

Thanks for the report.  I agree with your analysis, this should fix it.

-- 
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=3171

--- Comment #2 from Ron Frederick <ronf at timeheart.net> ---
Looks good - thanks for the quick response!

-- 
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=3171

Darren Tucker <dtucker at dtucker.net> changed:

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

--- Comment #3 from Darren Tucker <dtucker at dtucker.net> ---
Thanks, patch has been committed and will be in 8.4.

-- 
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=3171

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #4 from Darren Tucker <dtucker at dtucker.net> ---
Mass close of all bugs fixed in 8.4 release.

-- 
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.