[Nut-upsdev] Harden NUT work with strings where dynamic formatting strings are used

[Jim Klimov]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/nut-upsdev/attachments/20240603/01cf4639/attachment.htm>