Hello all, During discussion for development of a new driver, an old thought came to my attention that we have a potentially insecure approach with some parts of the codebase working with string and "var arg list" manipulation, which use dynamic `char *` variables instead of fixed strings (or macros that expand to those).
There are typically good reasons in code to do so, such as to generalize the use of mapping tables, or carefully construct formatting strings from pieces during run-time, but warnings from static analysis in compilers also has a point that a mistake here would cause the code to reference wrong areas of memory/stack or interpret the owned addresses as wrong data types. Currently those warnings were hushed by use of pre-processor pragmas, but this only hides the possible errors under the rug. The issue was formally posted as https://github.com/networkupstools/nut/issues/2450 and a PR to fix it was proposed at https://github.com/networkupstools/nut/pull/2460 The general idea for the fix (in several added methods for our different use-cases) is to provide a fixed constant "reference" formatting string, which just lists the data types in order of printf-style method's variable list of arguments - this is a sequence that humans and compilers can well analyze by looking at code (e.g. static analysis during compilation), like they do for "normal" string operations. The "dynamic" formatting string (variable) is another argument to those methods, which we compare at run-time to the "reference" string and decide if they are equivalent as far as memory safety is concerned (i.e. that they would treat bytes on stack as signed or unsigned integers or doubles of specified width, or as strings or pointers). The NUT CI farm formally agrees with this code across many platforms and toolkits, but before merging this fix, I would rather have different people using their drivers and hardware build the branch and test the modified code -- to make sure that existing mappings still work and are not rejected by run-time checks due to a mismatch of e.g. `long int` variable and a `%d` conversion character in some applicable mappings and `%ld` in others. The changes are not only in drivers, but also in many of the "clients" and a bit in nut-scanner. Discrepancies would be currently exposed when debug verbosity level is "1" or more. As usual, https://github.com/networkupstools/nut/wiki/Building-NUT-for-in%E2%80%90place-upgrades-or-non%E2%80%90disruptive-tests can be a good start to get your custom copy of NUT built (with `git clone https://github.com/jimklimov/nut -b issue-2450 nut-issue-2450; cd nut-issue-2450 ; ...` to those instructions). Thanks in advance, Jim Klimov
_______________________________________________ Nut-upsuser mailing list Nut-upsuser@alioth-lists.debian.net https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/nut-upsuser
