Call for testing: OpenSSH 7.9

Zev Weiss zev at bewilderbeest.net
Mon Oct 15 14:59:15 AEDT 2018


On Wed, Oct 10, 2018 at 10:54:21PM CDT, Damien Miller wrote:
>
>Live testing on suitable non-production systems is also appreciated.
>Please send reports of success or failure to
>openssh-unix-dev at mindrot.org. Security bugs should be reported
>directly to openssh at openssh.com.
>

With git revision 797cdd9c8, all tests passed with on Void Linux, though 
I hit a number of -wformat-trunction warnings during compilation (gcc 
8.2).

    fmt_scaled.c: In function 'fmt_scaled':
    fmt_scaled.c:272: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:272:46: note: directive argument in the range [-9007199254740991, 9]
       (void)snprintf(result, FMT_SCALED_STRSIZE, "%lld.%1lld%c",
                                                  ^~~~~~~~~~~~~~
    In file included from /usr/include/stdio.h:873,
                     from /usr/include/bsd/libutil.h:49,
                     from ../includes.h:141,
                     from fmt_scaled.c:41:
    /usr/include/bits/stdio2.h:67: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 ());
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

>From my reading of the preceding code (lines 249-258), it looks to me 
like the value of fract should be squarely in [0, 9] at that point, so 
either I'm missing something subtle or gcc's value-range analysis is 
confused.  In reading the rest of fmt_scaled() I noticed it does call 
llabs(number) without any range checks, which is undefined if number is 
LONG_LONG_MIN, though for what it's worth another compile with a test 
for that condition inserted before the llabs() call still produced the 
same warning.  (After some further experimentation with similar code in 
a minimized context I'm somewhat more confident in my suspicion of a gcc 
bug.)


    sshkey.c: In function 'sshkey_format_cert_validity':
    sshkey.c:2762:42: warning: '%s' directive output may be truncated writing up to 31 bytes into a region of size between 24 and 55 [-Wformat-truncation=]
       snprintf(ret, sizeof(ret), "from %s to %s", from, to);
                                              ^~         ~~
    In file included from /usr/include/stdio.h:873,
                     from /usr/include/bsd/libutil.h:49,
                     from includes.h:141,
                     from sshkey.c:28:
    /usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output between 10 and 72 bytes into a destination of size 64
       return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            __bos (__s), __fmt, __va_arg_pack ());
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

With appropriate knowledge of strftime this one appears safe (at least 
within the expected lifespan of the earth), but treating it as an opaque 
function writing the 'from' and 'to' buffers the warning seems 
legitimate (formatting two 31-byte strings plus 10 more bytes into a 
64-byte destination buffer).


    hostfile.c: In function 'host_hash':
    hostfile.c:151:44: warning: '%s' directive output may be truncated writing up to 511 bytes into a region of size between 509 and 1020 [-Wformat-truncation=]
      snprintf(encoded, sizeof(encoded), "%s%s%c%s", HASH_MAGIC, uu_salt,
                                                ^~
          HASH_DELIM, uu_result);
                      ~~~~~~~~~                  
    In file included from /usr/include/stdio.h:873,
                     from /usr/include/bsd/libutil.h:49,
                     from includes.h:141,
                     from hostfile.c:39:
    /usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output between 5 and 1027 bytes into a destination of size 1024
       return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            __bos (__s), __fmt, __va_arg_pack ());
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Likewise, with sufficient introspection into __b64_ntop I think we can
conclude this is safe, but from gcc's perspective it might not be.


    sshconnect.c: In function 'check_host_key.constprop':
    sshconnect.c:1027:8: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size between 773 and 973 [-Wformat-truncation=]
            "The authenticity of host '%.200s (%s)' can't be "
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    sshconnect.c:1032:18:
            host, ip, msg1, type, fp,
                      ~~~~
    sshconnect.c:1028:20: note: format string is defined here
            "established%s\n"
                        ^~
    In file included from /usr/include/stdio.h:873,
                     from /usr/include/bsd/libutil.h:49,
                     from includes.h:141,
                     from sshconnect.c:16:
    /usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output 130 or more bytes (assuming 2377) into a destination of size 1024
       return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            __bos (__s), __fmt, __va_arg_pack ());
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This one's in a somewhat more complex context and I haven't fully 
analyzed it myself.


    ssh-agent.c: In function 'main':
    ssh-agent.c:1215:48: warning: '/agent.' directive output may be truncated writing 7 bytes into a region of size between 1 and 4096 [-Wformat-truncation=]
       snprintf(socket_name, sizeof socket_name, "%s/agent.%ld", socket_dir,
                                                    ^~~~~~~
    ssh-agent.c:1215:45: note: directive argument in the range [-2147483648, 2147483647]
       snprintf(socket_name, sizeof socket_name, "%s/agent.%ld", socket_dir,
                                                 ^~~~~~~~~~~~~~
    In file included from /usr/include/stdio.h:873,
                     from /usr/include/bsd/libutil.h:49,
                     from includes.h:141,
                     from ssh-agent.c:37:
    /usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output between 9 and 4114 bytes into a destination of size 4096
       return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            __bos (__s), __fmt, __va_arg_pack ());
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Similar to the hostfile.c and sshkey.c ones above, though I think this 
one might potentially be ticklable with sufficiently large values of 
TMPDIR?


    ssh-keygen.c: In function 'do_gen_all_hostkeys.isra.12':
    ssh-keygen.c:1074:41: warning: '%s' directive output may be truncated writing up to 1024 bytes into a region of size 1023 [-Wformat-truncation=]
       snprintf(comment, sizeof comment, "%s@%s", pw->pw_name,
                                             ^~
           hostname);
           ~~~~~~~~                           
    In file included from /usr/include/stdio.h:873,
                     from /usr/include/bsd/libutil.h:49,
                     from includes.h:141,
                     from ssh-keygen.c:15:
    /usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output 2 or more bytes (assuming 1026) into a destination of size 1024
       return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            __bos (__s), __fmt, __va_arg_pack ());
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

(And another similar instance at line 2892.)  I'm not aware of any 
static limit on user name length analogous to NI_MAXHOST, so it seems 
like these might need to be dynamically allocated for full generality, 
though failing that making the comment buffer a few bytes bigger would 
at least mollify the compiler.


    ssh-keygen.c:340:34: warning: '%s' directive output may be truncated writing up to 1024 bytes into a region of size 39 [-Wformat-truncation=]
          "%u-bit %s, converted by %s@%s from OpenSSH",
                                      ^~
    ssh-keygen.c:342:19:
          pw->pw_name, hostname);
                       ~~~~~~~~        
    In file included from /usr/include/stdio.h:873,
                     from /usr/include/bsd/libutil.h:49,
                     from includes.h:141,
                     from ssh-keygen.c:15:
    /usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output 36 or more bytes (assuming 1060) into a destination of size 61
       return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            __bos (__s), __fmt, __va_arg_pack ());
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This one is similar to the others in this file, though I'm not sure why 
the comment buffer is so much smaller here -- output format 
restrictions?  (If so, perhaps a "%.60s" printf format or something 
instead of a small buffer?)


I also still (as has been happening for me for a while) get a bunch of 
-Wimplicit-function-declaration warnings on various arc4random 
functions, but I'm guessing that's not openssh's fault.



Zev



More information about the openssh-unix-dev mailing list