Re: Call for testing: OpenSSH 7.9

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]

 



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@xxxxxxxxxxx. Security bugs should be reported
directly to openssh@xxxxxxxxxxx.


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

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev



[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux