bugzilla-daemon at mindrot.org
2020-May-28 03:59 UTC
[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.