openssh bugs - Aug 2017 - [Bug 2769] New: String truncation warnings in fmt_scaled

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

            Bug ID: 2769
           Summary: String truncation warnings in fmt_scaled
           Product: Portable OpenSSH
           Version: -current
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P5
         Component: sftp
          Assignee: unassigned-bugs at mindrot.org
          Reporter: cjwatson at debian.org

Created attachment 3042
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3042&action=edit
Help compiler prove that fmt_scaled snprintf fits

fmt_scaled.c: In function ?fmt_scaled?:
fmt_scaled.c:269:52: warning: ?%1lld? directive output may be truncated
writing between 1 and 17 bytes into a region of size between 0 and 5
[-Wformat-truncation=]
   (void)snprintf(result, FMT_SCALED_STRSIZE, "%lld.%1lld%c",
                                                    ^~~~~
fmt_scaled.c:269:46: note: directive argument in the range
[-9007199254740992, 9007199254740991]
   (void)snprintf(result, FMT_SCALED_STRSIZE, "%lld.%1lld%c",
                                              ^~~~~~~~~~~~~~
In file included from /usr/include/stdio.h:938:0,
                 from /usr/include/bsd/libutil.h:46,
                 from ../includes.h:141,
                 from fmt_scaled.c:41:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:64:10: note:
?__builtin___snprintf_chk? output between 5 and 40 bytes into a
destination of size 7
   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        __bos (__s), __fmt, __va_arg_pack ());
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is actually OK, I think, but the compiler can't quite prove it. 
The attached patch helps it do so by syncing up a condition with the
nearby comment and by adding an assertion.

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

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

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at zip.com.au
             Blocks|                            |2698

--- Comment #1 from Darren Tucker <dtucker at zip.com.au> ---
this function comes from OpenBSD's libutil and I'll have to see what
the policy on asserts in that is.


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2698
[Bug 2698] Tracking bug for OpenSSH 7.6 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=2769

--- Comment #2 from Colin Watson <cjwatson at debian.org> ---
If you can't do an assertion, then some approach like "if (fract <
0)
fall_over;" should have a similar effect in terms of allowing the
compiler to deduce correct bounds on fract.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2782


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2782
[Bug 2782] Tracking bug for OpenSSH 7.7 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.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2698                        |


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2698
[Bug 2698] Tracking bug for OpenSSH 7.6 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.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2852


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2852
[Bug 2852] Tracking bug for OpenSSH 7.8 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.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2782                        |

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
Move to OpenSSH 7.8 tracking bug


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2782
[Bug 2782] Tracking bug for OpenSSH 7.7 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=2769

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3042|0                           |1
        is obsolete|                            |
             Status|NEW                         |ASSIGNED
                 CC|                            |djm at mindrot.org
           Assignee|unassigned-bugs at mindrot.org |djm at mindrot.org
   Attachment #3141|                            |ok?(dtucker at dtucker.net)
              Flags|                            |

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
Created attachment 3141
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3141&action=edit
with 100% less assert(3)

This one does it without assert, by clamping fract to [0, 10).

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

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3141|ok?(dtucker at dtucker.net)    |ok+
              Flags|                            |

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

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #5 from Damien Miller <djm at mindrot.org> ---
This is fixed in HEAD and will be in OpenSSH 7.8:

commit 32e4e94e1511fe0020fbfbb62399d31b2d22a801
Author: Damien Miller <djm at mindrot.org>
Date:   Mon May 14 14:40:08 2018 +1000

    sync fmt_scaled.c

    revision 1.17
    date: 2018/05/14 04:39:04;  author: djm;  state: Exp;  lines: +5
-2;
    commitid: 53zY8GjViUBnWo8Z;
    constrain fractional part to [0-9] (less confusing to static
analysis); ok ian@

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

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
Close RESOLVED bugs with the release of openssh-8.0

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