On Saturday, 22 January 2022 09:03:12 CET Sven Eckelmann wrote:
The struct
alfred_change_interface_v0 -> ifaces[IFNAMSIZ * 16] may be
written to but not fully initialized. The interface name may be much
shorter than the buffer holding it. Same applies struct
alfred_change_bat_iface_v0 -> bat_iface[IFNAMSIZ] but to a lesser extent
because the buffer is smaller.
But strncpy writes n bytes (second parameter of n). So the name + n-
strlen(name) 0-bytes. I thought I've corrected my earlier statement about
strncpy but maybe I forgot it. So strlcpy will take care of always writing a
single 0-byte at the end of a non-0-length buffer - but is not writing more
than 1x 0-byte (so half of the buffer might be uninitialised). strncpy will
fill the remaining bytes with 0 but might end up writing NO 0-bytes when
the buffer was already full.
Thanks for highlighting this difference between strncpy() and strlcpy(). I see
your point.
But in your status patch, not all 16 name entries were
written. So it leaks
things from the stack and the receiver might parse things which are not
actually written by the sender. And your code was not explicitly making sure
that the buffer ends with a 0-byte.
The server status patch iterates over the list of interfaces and performs
individual strncpy() calls, so that strncpy() can't initialize the entire
buffer unless there are 16 interfaces to begin with. Ok!
Then drop this patch.
Kind regards,
Marek Lindner