This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Implement C11 annex K?

David A. Wheeler wrote:

any function that doesn't limit writing length (and doesn't allocate)
can produce a buffer overflow.

Sure, and that's also true for functions that *do* limit writing length. Even strlcpy can produce a buffer overflow, because it can be given the wrong pointer or size. None of these techniques guarantee well-defined behavior in all cases. It's question not of absolutes, but of relative advantages.

some strlcpy calls allow truncation, but that's the point: That is
the *worst* that happens with mistakes.

No, a mistaken use of strlcpy can lead to undefined behavior, both as mentioned above and (in a different way) as in the auth.c:486 example discussed earlier. Undefined behavior is worse than silent truncation.

We can argue about whether or not the reduction is *worth* it

That's not what I'm saying. I'm saying there's no empirical evidence that having programmers rewrite code to use strlcpy improves reliability significantly more than spending the same effort to improve reliability via standard functions and/or widely-used technology. This issue is independent of whether the extra reliability is worth the effort.

Certainly the evidence we've seen doesn't look good for strlcpy/strlcat: of the five uses examined, one had undefined behavior, one had silent truncation, and the other three didn't fix any bugs compared to standard functions. Admittedly this is a tiny set of cases, but still ... these cases were supposed to be evidence *for* strlcpy, and instead they lean *against* it.

a cast to unsigned doesn't always work,
it depends on the system architecture and C implementation.

It is not a problem for any platform that code runs on. To allay any concerns about hypothetical oddball platforms I suggest adding 'static_assert (INT_MAX < SIZE_MAX);' (or its pre-C11 equivalent, for older platforms). Hypothetical problem solved. Quite possibly OpenSSH is already making that assumption elsewhere anyway.

snprintf helps, but is that really the best that can be done?

No, snprintf can be improved. For example, the sprintf function family should be extended to support size_t rather than int values for sizes. This would fix an obvious botch in the traditional API, and would also solve the abovementioned hypothetical problem.

More generally, though, we should encourage static checks for these sorts of problems, instead of run-time checks or (worse) silent truncation.

It's clunky

That's a style argument, a time-sink I'd rather avoid.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]