Bugzilla – Bug 129937
3 array subscript out of range
Last modified: 2005-12-13 15:28:49 UTC
I just tried to compile package dnsmasq-2.22-3 with the Intel C compiler It said 1. rfc2131.c(199): warning #175: subscript out of range The source code is char save = mess->file[128]; but in file dnsmasq.h it says u8 chaddr[16], sname[64], file[128]; so file[ 128] does not exist. Suggest code rework. 2. rfc2131.c(236): warning #175: subscript out of range rfc2131.c(253): warning #175: subscript out of range Duplicates.
dnsmasq.h has u8 chaddr[16], sname[64], file[128]; u8 options[312]; So mess->file[128] is the same as mess->options[0] If the filename is exactly 128 characters long, it won't have a terminating NULL. Just before some code which assumes a terminating NULL, I do, mess->file[128] = 0; /* ensure zero term. */ and later, before sending the packet, options[0] gets restored to the value in save.
>So mess->file[128] is the same as mess->options[0] You are assuming that local variables are laid out in memory in the same order as they are written in the source code. There is no requirement in the C standard for this to be true. Accessing arrays outside their boundaries is undefined, meaning that anything can happen.
True, for local variables, but the objects on question are not local variables, they are structure elements. A compiler isn't free to mess with the in-memory layout of structures, the worst is can do is add padding for alignment reasons. If that happened, the code in question would still be OK, but dnsmasq wouldn't work, since that structure defines the layout of a DHCP packet "on the wire". Quote from the standard: [#12] Within a structure object, the non-bit-field members and the units in which bit-fields reside have addresses that increase in the order in which they are declared. A pointer to a structure object, suitably converted, points to its initial member (or if that member is a bit-field, then to the unit in which it resides), and vice versa. There may be unnamed padding within a structure object, but not at its beginning. It would be possible to add a pragma to force the structure to be packed (no padding), but that would be non-portable, and the structure is such that any sane compiler won't pad it anyway. (the layout is designed with this in mind, with no n-byte fields no on a n-byte boundary, etc. Those IETF people worry about these things.)
(In reply to comment #3) > True, for local variables, but the objects on question are not local > variables, they are structure elements. Thanks, I failed to spot that. If you wanted to access mess->options[0], why not say this in the source code, instead of using the undefined circumlotion mess->file[128] ? Both myself and the Intel C compiler would be a lot happier with that.
it's not my decision. I'm not the author. You can suggest to change that to the author of dnsmasq.
(In reply to comment #5) > it's not my decision. I'm not the author. Thanks for the tip. > You can suggest to change that to the author of dnsmasq. I would if I could. Suse got the bug report because I could not find the author's email address.
This bug still exists in version 2.24-2, around 13 Dec 2005.
I know. It's wanted that way by the author of dnsmasq and actually it's not a bug.