|
Bugzilla – Full Text Bug Listing |
| Summary: | 3 array subscript out of range | ||
|---|---|---|---|
| Product: | [openSUSE] SUSE LINUX 10.0 | Reporter: | David Binderman <dcb314> |
| Component: | Network | Assignee: | Uwe Gansert <ug> |
| Status: | RESOLVED INVALID | QA Contact: | E-mail List <qa-bugs> |
| Severity: | Minor | ||
| Priority: | P5 - None | ||
| Version: | Final | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | SuSE Linux 10.0 | ||
| Whiteboard: | |||
| Found By: | Other | Services Priority: | |
| Business Priority: | Blocker: | --- | |
| Marketing QA Status: | --- | IT Deployment: | --- |
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. |
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.