If you are writing a program in C without a a real string API then it is likely your program will end up on a list like this, and all programs need to care about security and/or reliability. As a somewhat graphic example of this, consider the following two groups of programs found on many Linux machines... ¶
| vsftpd | wu-ftpd |
| postfix | sendmail |
| djbdns | bind |
| And-httpd | LPRng |
| cups | |
| nfs | |
| vixie-cron | |
| squid | |
| apache-httpd | |
| openssh |
...the left column all have had zero remote buffer overflow attacks, three of the four have had "security guarantees" where the authors were so sure they were secure money was offered for anyone who could prove otherwise. They all consistently use a dynamic string API.
While the right column have all had at least one remote buffer overflow and try to use just the standard C-style string.h functions, and possibly a couple of helpers in the vein of strdup() ... this is far from enough.
The "common" reasons for not using a string library include: 1) You "just have to be careful" and you'll be alright. 2) It is too slow to use one. ¶
The first argument just ignores reality, the squid maintainers are good but ... they still missed one, the apache team are careful and have code inspections, but ... they still missed one, and in neither case was it "obvious" by looking at the code that there would be a problem. For a list of recent security bugs against Red Hat Linux, and an indication of how many wouldn't exist with a string API see this page.
Humans just don't do that well with repeated tasks, in fact I'd say that given a formula of "do X marginally difficult thing Y times" X has to get smaller exponentially as Y gets bigger ... and when dealing with string types in code Y gets very big. The only tenable solution is to make X be 0. The way to do that is to have all of your string operations go through an API that guarantees you can't overflow buffers. This means that for the API X is comparatively large but Y, the problem of not overflowing a buffer, is reduced to 1. Whereas for each call to the API (which is a lot so Y is big), X is zero.
So given that you want a good string API, then the quickest/safest way to get one is just to reuse one that is known to do what you want. There is a good list on this page. However there are certain things that you should look for when evaluating which library to choose (or possibly writing your own, although I'd heavily recommend against that). Note that, as I said on the string library API comparison page security is somewhat subjective so you must think about your situation. It's possible that IO is much less important to you than having a simple sprintf that always aborts() on '%n'. Or that sprintf won't be used, and so that important to you.
The speed argument is discounted pretty well by the information here.
Some APIs will use statically sized buffers, and limit the size of data that goes into that buffer. This isn't enough as Y in the above formula can still grow due to the fact that you can always have problems like...
¶
/* broken example when using a limiting string type API */
API_TYPE s1;
...
API_add(s1, "abcd ");
API_add(s1, external_data);
API_add(s1, " xyz");
...where both "abcd " and " xyz" need to be in s1 at the end, of course the above will seem to "work" until one day someone makes external_data too large and then anything can happen. The other problem is that this is almost always implemented with the data (the string) and the metadata (it's length) passed to the functions separately, this creates some great code like...
¶
/* broken example */
char *s1;
size_t l1;
...
API_add(s1, l1, "abcd "); l1 -= strlen("abcd ");
API_add(s1, l1, external_data); l1 -= strlen(external_data);
API_add(s1, l1, " xyz"); l1 -= strlen(" xyz");
...which looks OK at first glance, but of course the middle add didn't add strlen(external_data) it added strlen(external_data) or l1 ... whichever was smaller. So, due to underflow, you get the same affect as just having no limit at all.
Or code like...
¶
/* broken example ...
* code supposedly OK because the input and the output are the same
* maximum size */
char buf1[X];
char buf2[X];
char buf3[X];
char *s1 = buf1;
char *s2 = buf2;
char *s3 = buf3;
char *last = s2;
...
/* find biggest shared prefix, and skip that */
while (*s1 && (*s1 == *s2))
{
if (*s2++ == '/')
last = s2;
++s1;
}
/* relative point to shared part of path */
while (*s1)
{
if (*s1++ == '/')
{
memcpy(s3, "../", strlen("../"));
s3 += strlen("../");
}
}
strcpy(s3, last);
And, again, the above code seems to work fine. And even looks reasonable, you are swapping a directory "/a/" for "../" and they are the same number of characters ... until someone thinks about two directories where "/a/b/" becomes "../../", and then it can be seen that you now have 1 more in the destination than in the source.
Or code like...
¶
/* broken example ...
* code supposedly OK due to count */
int func(char *dst, size_t len, const char *src)
{
while (*src)
{
const char *tmp = NULL;
if ((src[0] == '$') && (src[1] == '{') && (tmp = strchr(src, '}')))
{ /* sub a variable */
size_t var_len = (tmp - (src + 2));
size_t sub_len = 0;
src += strlen("${");
if ((tmp = lookup(src, var_len)) && (sub_len = strlen(tmp)))
{
if (sub_len > len)
sub_len = len;
memcpy(dst, tmp, sub_len);
len -= sub_len;
dst += sub_len;
}
src += var_len + 1;
}
else /* no substitution */
*dst++ = *src++;
}
if (len)
{
*dst = 0;
return (1);
}
return (0);
}
And, again, the above code seems to work fine, until somone sees that the length isn't shortened on non-substitutions.
Or code like...
¶
/* broken example ...
* code supposedly OK because the input and the output are the same
* maximum size */
int func(const char *src)
{
char buf[X];
unsigned int count = 1;
strcpy(buf, src);
strtok(buf, ",");
while (strtok(NULL, ","))
{
++count;
}
return (count);
}
/* callsite 1... */
char buf[X];
func(buf);
/* callsite 2... */
func("a,b,c");
/* callsite N... */
func(tmp);
And, again, the above code seems to work fine, until someone sees that "tmp" can be bigger than X bytes.
Remember that if you had a good allocating string API, none of the above errors would be possible ... and the kind of errors that would be possible are much more likely to happen all the time, they don't seem to work and then fail horribly on the "corner cases".
It's also worth noting that fixed size buffer APIs often use more memory to store the same amount of data (and with a clever allocating API like Vstr it is pretty much guaranteed that they'll take much more space).
Some APIs will represent strings as an array of characters followed by a '\0' (aka. NIL) character. While saving you a theoretical maximum of 4 bytes of metadata, on a 32bit system or 8 on a 64bit system, (often this doesn't happen due to overhead in the malloc() implementation though) you now have the problem of what happens when you read your data from a file or over the network and the data itself contains a NIL character. Of course as you read the data you can convert all NIL bytes (slow), or you can add the data to the string API and check the old length and the added length against the APIs idea of the new length. However this is both somewhat slow (remember it has to get the length by calling strlen()) and prone to error (Y goes up).
Also you should look to see if the API has IO functions supplied, strings are sometimes used internally to compare/search against other strings but often have to goto or come from some outside resource. If you are having to write your own functions to move your data between the string API and the network/file system/etc. then those are all sources of potential security problems. Most of the average and better libraries provide at least good blocking IO support.
Of course where you need this common functionality done for you most often, is when it's most difficult ... and that's when you have multiple forms of IO going on at once, and can't block on one at the expense of the others. For OK non-blocking IO support, the library should provide at least a way to do multiple reads to the string appending data if it is available and a way to do a write of the string writing as much as possible and then being able to store how much was written so that it can be resumed later when there is more room on the network connection/whatever.
Also note that if the library provides a read IO operation it must either:
...if it doesn't do any of these it is worthless, as it'll just corrupt it's own input. If it just gives you some way to work out the data is corrupt, for instance it keeps a length as metadata but all the other functions for searching/comparison/etc. assume a C-style string, then you need to be extremely careful as each "supplied" read IO function is a danger on it's own but there is no way of enforcing that people call a conversion function afterward.
Solving problems is what your code is for, solving string problems is what the string API is supposed to be for ... so if you need to search for data, compare data, split data into tokens or substitute X data with Y data inside a string you shouldn't have to write that code yourself. It's just too common, and can too often lead to accessing or writing to data out of bounds of the string.
Of course this doesn't mean that a larger API is always better, in fact it's better if you just have to remember a few functions that you need to use. But as occasionally you'll want a higher level string related operation, such as decoding a URI or splitting a string ... if you find yourself having to write it then that is bad.
Not much needs to be said about this. Testing is the process by which you prove code isn't full of bugs, therefore if there is no obvious testing ... the code is full of bugs. It is also true that the simpler the implementation the more likely it is to have less bugs, ergo. the more efficient/bigger the implementation the more testing should be provided.
The final check is to make sure that the testing code tests both the default path of the code you are interested in (as it doesn't matter if the code you dont' use gets tested 1,000 times if you are do use isn't tested at all), and the corner cases of said code (malloc() error paths, open() failure paths, read()/write() failure paths, and substituting larger/smaller values etc.).
Almost all C programmers use the functions in the printf() family on a regular basis, and if you want to do i18n formatting of output then doing it using the POSIX i18n argument modifiers to the printf() family of functions means that half of the job is done for you. So, in short, having a safe and portable version of sprintf() for your chosen string API is something that will pay off.
The first thing to consider is that just using the default sprintf()/snprintf() functions isn't a good idea. This problem also arises with naive string APIs which just call directly to the host implementation.
The next thing to consider is that having some string data, and then wanting to append some data to it that comes from a printf() like function is common enough that the string API really needs to provide a printf like function variant that does this (indeed it could be the only implementation, assuming there is an easy way to delete all). If the string API doesn't provide an append variant, then it's very likely that the programmer will try and make something work on their own ... which is open to problems.
The final thing is an easy way to put non-fundamental types in the printf "language". The most obvious need is to output strings of the string APIs type. Eg.
¶
APT_string *s1;
APT_string *s2;
...
API_add(s1, data);
API_fmt(s2, "%s %d %{API_string}", "abcd", 4, s1);
However extensions for printing IP addresses or other types of data can be very useful as well, and it is even better if there is a way to "register" callback functions to format user supplied data. The biggest obstacle here is static format checkers, like gcc, which will give an error on the above code due to %{ not being a recognized format. The common solution to this is to ignore it, which makes the feature very hard to use. Vstr uses a system that lets the static checker into skipping over the bits it doesn't understand, which while not perfect (you can still typo the formatter name) at least makes sure that the format string won't do anything worse than format the output incorrectly (as opposed to crashing, or causing a security hole). However, as far as I know, Vstr is the only string library to offer this.
Just as with the biggest problem in the C-style string.h API, a string API shouldn't just silently die if you pass it the same string for both an input and an output. It is possibly acceptable to have two kinds of functions for every operation, one where the strings are assumed to not overlap and one where they can. However I'd strongly suggest that you just use a string API that does the single if check for you.
Note however that it's probably prohibitively expensive for the string API to do this checking, if you just give it internal data and not the string object itself.
The standard C string functions have a number of problems, some of which you'll see above as things you should watch for when evaluating other string APIs and some of which are somewhat "unique" to the standard C functions.
ISO 9899:1999 says this in the last sentence about memcpy() (at 7.21.2.1)...
If copying takes place between objects that overlap, the behavior is undefined.
...where "undefined" is std. speak for broken. The safe way to do this is seen in the last sentence about memmove() (at 7.21.2.2)...
Copying takes place as if the n characters from the object pointed to by s2 are first copied into a temporary array of n characters that does not overlap the objects pointed to by s1 and s2, and then the n characters from the temporary array are copied into the object pointed to by s1.
...both of which most C programmers are at least vaguely aware of, due to the two functions doing the same thing apart from that single difference. However a lot of C programmers aren't aware that strcpy(), strncpy(), strcat(), strncat() and sprintf() all follow the memcpy() behavior for overlapping objects. So the following code is broken...
¶
/* broken example ... common mistake */
char *s1;
...
strcpy(s1, s1 + 1); /* chop the first character from the C string */
...as is the following...
¶
/* broken example ... fairly common mistake */
char *s1;
int X;
...
sprintf(s1, "%s:%d", s1, X); /* append a number */
...it also means that there is no way to copy n bytes within a string using the std. string functions. You have to resort to calculating the right lengths and calling memmove().
If that wasn't enough, strncpy() the std. function that most C programmers use to safely copy no more than N characters to a string is all but useless in all conditions. Consider...
¶
#define SZ 4
char s1[SZ];
strncpy(s1, "abcd", SZ); /* 1 */
strncpy(s1, "abc", SZ); /* 2 */
strncpy(s1, "a", SZ); /* 3 */
...in case 1 strncpy() copies {'a', 'b', 'c', 'd'}, which means that the string isn't terminated and will almost certainly cause problems very soon, even strncat() doesn't work in this case and that's described on the next page. In case 2 strncpy() copies {'a', 'b', 'c', '\0'}, which is exactly what memcpy() would have done ... but it does it slower. In case 4 strncpy() copies {'a', '\0', '\0', '\0'} which is a nice 100% overhead, that is useless to pretty much everybody. Of course normally the array have a much bigger size than 3, so the overhead can be 25,500% (a 16 byte filename into a PATH_MAX array).
The problem in case 1 means that it's common to see code calling strncpy() do either...
¶
char s1[SZ];
...
strncpy(s1, external_data, SZ - 1);
s1[SZ - 1] = 0;
...or...
¶
char s1[SZ + 1];
...
strncpy(s1, external_data, SZ);
s1[SZ] = 0;
...or...
¶
static char s1[SZ + 1];
...
strncpy(s1, external_data, SZ);
All of which are a source of bugs, when one of the values is even slightly wrong. The second is particularly sad, as the reason it's done that way is to improve performance (using an interface that can cause over 25,000% overhead, in the common case).
Note that the last example listed above...
¶
static char s1[SZ + 1];
...
strncpy(s1, external_data, SZ);
...works by "getting rid" of the problem (not remembering to write NIL terminator) by "knowing" that the last byte will always be a NIL and so not overwriting that. However this is a very big trap for those that don't intimately know what is happening, due to...
¶
/* broken example */
char s1[SZ + 1];
...
strncpy(s1, external_data, SZ);
...or...
¶
/* broken example */
char *s1 = malloc(SZ + 1);
...
strncpy(s1, external_data, SZ);
...being errors.
It's also worth noting that two very recently introduced interfaces have been included in a number of systems: strlcpy() and strlcat(). Neither of these are specified to fix the copying inside a string problem, and both can cause different problems as they assume that all input is NIL terminated. However they aren't specified by anything more than their source, which is different for a few of the implementations. This has already led to the Solaris version acting differently than the original OpenBSD version, and a broken implementation in an O'Reilly article.
The original specification for the C run time only specified the function sprintf() as an easy way to put formatted data into a C-style string. The obvious problem was that you always have a limited amount of space allocated, so to make sure you don't overflow the buffer the "extension" snprintf() was added ... or I should say at least 3 different implementations of an extension called snprintf() was added. The differences centered around what the function should return. The choices being: 1) The value sprintf() would return, Ie. the space needed. 2) The value sprintf() would return or the limit, whichever is smaller. 3) The value sprintf() would return if it is smaller than the limit, and -1 if it is not. All of these different return values have to be dealt with by a portable program, or you can have buffer overflows (esp. in the second case). This is especially a problem due to the fact that a lot of uses of snprintf() are so that people can implement a function that allocates the correct amount of storage for sprintf() and then call that, because doing blanket limiting is a bad idea. The ISO 9899:199 standard chose the first choice (the value sprintf() would return), so now we just wait 5 years for most systems to have the standard semantics.
This led to implementations offering an extension called "asprintf()" which will allocate the correct amount of space and sprintf() into it. However the interface had to be created to be able to return an error separate from the obvious "could not allocate memory" (mainly due to wchar_t conversions, which hardly anyone cares about) defying all previous experience with snprintf() and all expectations of the average programmer the version of the interface shipped with most Linux distributions is both different to every other implementation and is the only interface that returns a pointer to memory that you cannot use (this has been changed in a CVS commit on 2004-06-06). For instance the following code works everywhere but on those glibc based Linux systems, where it is a security problem...
¶
/* broken code, on Linux only */
char *s1;
int off;
asprintf(&s1, "%d:%n%s,", strlen(data), &off, data);
if (!s1) goto error;
...
if (strlen(other_data) == strlen(data))
strcpy(s1 + off, other_data);
else
{
free(s1);
asprintf(&s1, "%d:%s,", strlen(other_data), other_data);
}
...due to a strcpy() buffer overflow and an invalid value passed to free().
So here's a quick quiz, what does "ret" contain in the following code...
¶
char buf[4];
int ret = snprintf(buf, sizeof(buf), "%.*s", 14, "\x80\x80\x80\x80 123456789");
...of course after the discussion about what the return value should be for overflow you'll have said depends on the implementation. But let's assume that in the following code "ret" is 14 (Fourteen)...
¶
char buf[4];
int ret = snprintf(buf, sizeof(buf), "%s", "\x80\x80\x80\x80 123456789");
Ah, you say "it follows the ISO 9899:1999 behaviour" so the value of ret must be 14 (Fourteen). However, you'd be wrong, the value returned is -1 (Minus one) on Linux if you are in a UTF-8 locale due to the multibyte "issue". More generally ISO 9899:1999 says:
7.19.6.5/3
The snprintf function returns the number of characters that would have been written had n been sufficiently large, not counting the terminating null character, or a negative value if an encoding error occurred. Thus, the null-terminated output has been completely written if and only if the returned value is nonnegative and less than n.
Also note that the std. declares that mixing use and non use of i18n parameter numbers is "undefined" in std. C ... which is great for the libc developers, but not so much fun from a security point of view.
You also have to remember that there is no way to tell what version of a sprintf() routine you are going to be using, or if it is as bad a quality as say the dietlibc implementation. So you have all of the problems that other string APIs have with re-implementing a printf like function, but no way to tell which you are using (and test against them) when you write it.