On 08/02/2018 12:15 PM, Bernd Edlinger wrote:
On 08/02/18 19:00, Martin Sebor wrote:
As an alternate approach I have been thinking about, if
there is a strong feeling that allowing strlen to iterate
past the subobject boundary is necessary (I don't believe
it is.)
Rather than indiscriminately expanding the provenance of
the subobject regardless of what members follow it in
the enclosing structure, only consider doing that if
the next member is an array of the same type. E.g.,
struct S { char a[4], b[3], c[2], d; };
extern struct S *p;
strlen (p->a); // consider p->a's bounds to be char[9]
No, initially I thought in the same direction,
but looking at the way how X-server is broken,
I realized that will probably not be sufficient.
Maybe it would be good to have one set of optimistic range infos
that follow the standards, and can be used to control the warnings,
and another set of pessimistic range infos, that control optimizations.
Consider
extern char garbage[10];
char x[10];
memcpy(x, garbage, 10);
x[9] = 0;
strlen(x) will be 0..9 no matter what was in garbage.
That information can be safely used for optimizations.
But if we have
extern char garbage[10];
char x[10];
memcpy(x, garbage, 10);
char *y = x;
if (strlen(y) < 10) <= may not be removed pessimistically
This example involves a whole object. There should be no question
that running off the end of a full object is undefined and a bug.
It's far preferable to avoid such bugs. Emitting a library call
that can return an arbitrary value or crash is the least secure
and least user-friendly solution.
char z[10];
sprintf(z, "%s", y); <= omitting warning would be okay optimistically.
It is not really easy to do but possible.
I see three cases here:
1) y points to a constant array whose initializer we know
2) y points to an array with contents (length) known to
the strlen pass
3) we don't know what y points to or its contents
(1) can be handled easily by extending the approach in my patch
for bug 86552 to other built-ins. I have a patch that does that
for sprintf.
(2) can be handled by extending the strlen pass to also track
the sizes of array objects into which it tracks stores. I'm
working on a patch for GCC 9.
(3) can only be handled by adding an even stricter setting
for -Wformat-overflow by warning for all %s arguments of
unknown length. I haven't considered this yet.
In the moment I would like to concentrate exclusively on wrong code issues
The only wrong code is in programs that run off the end of
unterminated buffers. Those are the ones I believe we should
focus on helping detect and prevent. Warnings are the fist step.
Enhancing _FORTIFY_SOURCE and the sanitizers to also detect such
reads would be another solution (in addition to warnings).
Emitting traps (say under an option) would be yet another. But
changing GCC to silently accept such programs is, in my opinion,
the worst possible approach.
Martin
