On Sun, 2013-09-22 at 02:09 -0500, Steve M. Robbins wrote:
> On September 21, 2013 09:04:23 PM Bernhard R. Link wrote:
> > * Kees Cook <kees@debian.org> [130921 17:08]:
> > > In a theoretical sense, sure. In this particular case, why bother breaking
> > > it when it's a trivial 1 line fix? My original approach was to fix it in
> > > libc and do a mass bug filing. Everyone wins. If we want to reject the
> > > undefined behavior, we should modify the compiler to reject it. Seems to
> > > me
> > > it's a bug to even allow undefined behavior.
> >
> > The whole point of undefined behaviour in C is that the
> > compiler/implementor/... does not have to care.
>
> I strongly suspect the "whole point" of undefined behaviour is simply that at
> least two parties on the committee simply couldn't agree on "correct"
> behaviour.
No, in that case the standard may say there is 'implementation-defined'
behaviour (several possibilities allowed, must be documented) or
'unspecified' behaviour (several possibilities allowed, does not need to
be documented).
'Undefined behaviour' is usually stated where it is thought to be
impractical for all implementations to detect and specifically handle
the error case.
> > Checking every time would
> > make it slower,
>
> What are you referring to as "it"? The compiler? Checking that two arguments
> to a function are the same doesn't strike me as terribly expensive.
But how about this - does it result in overlapping reads and writes?
sprintf(buf, "%s", buf + 4);
Obviously, it will depend on the length of the string starting at buf +
4. As I said, it is impractical to detect all such errors (it is
possible but at a significant performance cost).
> > requesting any specific behaviour would make it slower.
>
> Nonsense -- it has a specific behaviour now. Since the standard says it is
> undefined, there's nothing stopping us from reverting back to its old behaviour
> which, arguably, better mached people's expectations -- else they wouldn't
> have written the "buggy" code. Moreover, it is the same behaviour used when
> NOT compiled with _FORTIFY_SOURCE=2.
I agree that library implementers should care about run-time regressions
even for buggy applications. However, in this case, failing to clear
the buffer may undermine the static analysis of buffer size (I haven't
thought too carefully about whether that's the case).
Given that it's a reasonably common error to try to sprintf() a string
over itself, it might be worth adding an assertion (or even better, a
static check) to catch it.
Ben.
--
Ben Hutchings
compatible: Gracefully accepts erroneous data from any source
Attachment:
signature.asc
Description: This is a digitally signed message part