Re: Secure C library

I read much of the new "security TR", and gee, I don't know. I guess
it is a step forward, but it seems like it doesn't go far enough.
It seems like it could be criticized for failing to learn the lessons of
the buffer overrun problem. It's not as broken as the current buffer-
and string-handling APIs, but it seems to miss an opportunity to do a
lot better.

For instance, it perpetuates the tradition of separating the pointer to
the buffer from the buffer size. I think that is a mistake. One of
the lessons of the buffer overrun fiasco is that buffers should have
been given an opaque type (an abstract data type), which internally
should have contained both the size of the buffer (the number of bytes
allocated) as well as a pointer to the buffer -- and all operations on
the buffer should check that size. The Java String class gets this right;
the new "security TR" gets it wrong.

It is not hard to design a better form of buffer and string handling.
For instance, the header files might define an opaque buffer type:
// invariant: buf.contents points to an object with buf.size bytes avail
typedef struct {
size_t size;
char *contents;
} buf_t;
Then one might define functions that allocate, deallocate, copy, and
otherwise operate on buffers. One could then build on top of this a
string interface that supports all the usual string operations, but
where these operations are automatically bounds-checked (thanks to the
presence of the .size field). Programmers could be encouraged not to
"peek inside" the struct, except possibly in those rare cases where
direct access is absolutely necessary.

The benefit of an opaque type like this is that there is less opportunity
for error. Security errors where the wrong size is passed in just
go away. There are many possible variations on this idea, of course,
but this is just one example of how thoughtful interface design can
contribute to reducing the likelihood of programming errors.

I'm also disappointed that the TR still permits undefined behavior.
Undefined behavior is a menace to security, and probably should have
been eliminated.

It probably would have been better if strings were represented in
length-buffer format, rather than as '\0'-terminated buffers. The problem
with the latter is that they cannot represent strings containing '\0'
bytes, and this has caused security problems in the past when interfacing
with other systems that interpret their strings differently.

Every instance of "char" should probably have been "unsigned char", or
there should have been a requirement that "char" be unsigned, to help
programmers avoid signed/unsigned bugs.

tmpfile_s() is nice, a good addition.

tmpnam_s() seems problematic, because of TOCTTOU problems with the
interface itself. My man page for tmpnam(3) says:
DESCRIPTION
The tmpnam() function returns a pointer to a string that is a valid
filename, and such that a file with this name did not exist at some
point in time, so that naive programmers may think it a suitable name
for a temporary file.
BUGS
Never use this function. Use mkstemp(3) instead.
The same criticisms seem to apply equally to tmpnam_s(), as the
"Recommended practice" implicitly hints at (but does so in a very weak
way: "tmpfile_s should be used instead of tmpnam_s *when possible*"?
whatever).

The *scanf_s() functions should have banned %n. I'm not sure what
the rationale for allowing %n with these functions is; they are just
as dangerous as the *printf style function. The *scanf_s() functions
probably should also ban %p, as that is unsafe for input.

Why does the standard define gets_s(), if the recommended practice is
to use fgets() instead of gets_s()? gets_s() can easily truncate lines
and lead to incorrect behavior. Isn't gets_s() just asking for trouble?

getenv_s() should probably specify that it is a constraint violation if
'name' appears in the environment more than once (as this route has been
used in real attacks).

Some functions like strcpy_s() allow crazy things to happen if their
inputs overlap. This should have been specified as a constraint
violation. The same goes for strncat_s().

I don't understand why strncpy_s() exists. It seems pointless, given
the existence of strcpy_s(). String truncation has led to security holes
in the past. It is rare that silent truncation is the desired behavior.
The same goes for strncat_s().

In general, the interface doesn't seem like it has been designed; it
seems like someone just took the existing string-related functions and
tweaked them with a max str len. But some of the old unbounds-checked
string functions don't make much sense once you add bounds-checking
(if they ever did). It seems like if one had put some thought into
designing an interface just for the purpose of reducing the frequency
of security bugs, and studied the history of problems in the current
interfaces, one could do better.
.

Relevant Pages