| Summary: | String truncation warnings in fmt_scaled | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | Colin Watson <cjwatson> | ||||||
| Component: | sftp | Assignee: | Damien Miller <djm> | ||||||
| Status: | CLOSED FIXED | ||||||||
| Severity: | normal | CC: | djm, dtucker | ||||||
| Priority: | P5 | ||||||||
| Version: | -current | ||||||||
| Hardware: | Other | ||||||||
| OS: | Linux | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 2852 | ||||||||
| Attachments: |
|
||||||||
this function comes from OpenBSD's libutil and I'll have to see what the policy on asserts in that is. 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. Move to OpenSSH 7.8 tracking bug Created attachment 3141 [details]
with 100% less assert(3)
This one does it without assert, by clamping fract to [0, 10).
This is fixed in HEAD and will be in OpenSSH 7.8: commit 32e4e94e1511fe0020fbfbb62399d31b2d22a801 Author: Damien Miller <djm@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@ Close RESOLVED bugs with the release of openssh-8.0 |
Created attachment 3042 [details] 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.