| Message ID | 20220824081415.21785-1-ynezz@true.cz |
|---|---|
| State | Superseded |
| Delegated to: | Petr Štetiar |
| Headers | show |
| Series | [rpcd] sys: mitigate possible strncpy string truncation | expand |
Hi, comment below. ~ Jo On 8/24/22 10:14 AM, Petr Štetiar wrote: > [...] > --- a/sys.c > +++ b/sys.c > @@ -224,7 +224,8 @@ procstr: > continue; > > if (!strcmp(var, "Package:")) { > - strncpy(pkg, p1, sizeof(pkg)); > + strncpy(pkg, p1, sizeof(pkg) - 1); > + pkg[sizeof(pkg) - 1] = '\0'; I suggest to change declarations of `pkg` and `ver` to: char pkg[128] = { 0 }, ver[128] = { 0 }; This way you can omit this explicit '\0' assignment since the dest char array will be zero initialized and the `sizeof(...) - 1` above will ensure that the last '\0' is never overwritten. > continue; > } > > @@ -241,7 +242,8 @@ procstr: > } > > if (!strcmp(var, "Version:")) { > - strncpy(ver, p1, sizeof(ver)); > + strncpy(ver, p1, sizeof(ver) - 1); > + ver[sizeof(ver) - 1] = '\0'; > continue; > } > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel >
diff --git a/sys.c b/sys.c index 097e7057f7da..45b15746d93f 100644 --- a/sys.c +++ b/sys.c @@ -224,7 +224,8 @@ procstr: continue; if (!strcmp(var, "Package:")) { - strncpy(pkg, p1, sizeof(pkg)); + strncpy(pkg, p1, sizeof(pkg) - 1); + pkg[sizeof(pkg) - 1] = '\0'; continue; } @@ -241,7 +242,8 @@ procstr: } if (!strcmp(var, "Version:")) { - strncpy(ver, p1, sizeof(ver)); + strncpy(ver, p1, sizeof(ver) - 1); + ver[sizeof(ver) - 1] = '\0'; continue; }
gcc 10 with -O2 reports following: In function ‘strncpy’, inlined from ‘rpc_sys_packagelist’ at /opt/devel/openwrt/c-projects/rpcd/sys.c:244:4: /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 128 equals destination size [-Werror=stringop-truncation] 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In function ‘strncpy’, inlined from ‘rpc_sys_packagelist’ at /opt/devel/openwrt/c-projects/rpcd/sys.c:227:4: /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 128 equals destination size [-Werror=stringop-truncation] 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Since it is not possible to avoid truncation by strncpy, it is necessary to make sure the result of strncpy is properly NUL-terminated and the NUL must be inserted explicitly, after strncpy has returned. References: #10442 Reported-by: @alexeys85 Signed-off-by: Petr Štetiar <ynezz@true.cz> --- sys.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)