[Bug stdio/28989] __snprintf_chk bounds check is too strict

["carlos at redhat dot com" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=28989

Carlos O'Donell <carlos at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |carlos at redhat dot com

--- Comment #3 from Carlos O'Donell <carlos at redhat dot com> ---
I just reviewed the language in the upcoming C2x standard (N2454) and it
remains the same in that the exact language for snprintf is ambiguous about
what happens for the bytes between the actual size of 's' and the point at
which characters start being discarded i.e. 'n-1'.

It is in my opinion a weak argument to say that because C2x is ambiguous that
we should relax the check in __snprintf_chk.

POSIX is clearer in that it states that 'n' is the size of the buffer in 's',
rather than just the point at which discarding starts to happen.

_FORTIFY_SOURCE should allow a strict bounds to catch defects where writes
occur beyond 's' but before 'n-1' which may or may not be out-of-bounds. Yes,
there may be a *practical* false-positive here if the format specifier would
not write more than the actual size of 's', and the caller did not want to
adjust 'n' because it had a runtime cost. In this case I see two scenarios:

(a) Caller doesn't adjust 'n' to match size of 's' because of runtime cost, and
relies on specifier to control how much was written to 's'.
- This is risky since in the future the specifier may change.

(b) __snprintf_chk becomes more expensive to avoid the case where another
restriction (format specifier) would prevent the overflow from happening.

Again, this seems like a weak argument. Relying on the format specifier, which
is in no way directly coupled with 'n' is a recipe for a potential future bug.

The caller may not know the size of 's', in which case it should not use
snprintf since there is no inherent benefit.

I agree with Andreas here, we can be stricter in __snprintf_chk, it is both
cheaper, and safer.

-- 
You are receiving this mail because:
You are on the CC list for the bug.