5 ms·

strl(cpy|cat)() is quite dangerous to just use blindly as a full replacement for other (mem|str)*(cpy|cat) functions. strlcpy's return value is the length of th

by byuu 13y ago

strl(cpy|cat)() is quite dangerous to just use blindly as a full replacement for other (mem|str)*(cpy|cat) functions. strlcpy's return value is the length of the source string. So say you have a big 20MB XML document, and as you are scanning along, you use strlcpy() to store a node name or value into another string. Even though you only ask it to copy five bytes, it goes ahead and runs over the rest of the 20MB document simply for a return value you don't (in this case) care about. If you keep doing this over the entire document, your parser will end up with quadratic overhead. strlcat() has the same overhead issue, and its return value is downright cryptic: it is strlen(source) + min(length, strlen(target)). When it tries to compute strlen(), ignoring your requested length parameter, this means it will access out-of-bounds memory if that string is not properly null-terminated. And if you're absolutely paranoid about security, these functions not having both source size and target size parameters gives you a second way to go out-of-bounds. Overall, they're okay functions, but you can easily do a lot better. And since these aren't universally available, if you're going to declare your own cpy/cat functions, you may as well use a better implementation than strl.

clarry 13y ago
> strl(cpy|cat)() is quite dangerous to just use blindly Very true; people have caused serious issues by blindly doing things (supposedly making things secure or else), without bothering to understand what they're doing. However, for the specific (and reasonably common) scenario of dealing with proper C strings, the strl* functions are very simple and easy to use correctly, and their use is easy to audit. > So say you have a big 20MB XML document, and as you are scanning along, you use strlcpy() to store a node name or value into another string. [..] If you keep doing this over the entire document, your parser will end up with quadratic overhead. Right. It is going to be ridiculously slow (and that should be obvious). But this isn't the common scenario that strl* family of functions were designed for (and which a lot of people get wrong with standard C string functions). But chances are in code like this C strings' shortcomings would be problematic anyway. > strlcat() [..] its return value is downright cryptic: it is strlen(source) + min(length, strlen(target)). Sorry but that is just wrong. > When it tries to compute strlen(), ignoring your requested length parameter, this means it will access out-of-bounds memory if that string is not properly null-terminated. It's not "requested length", it's "this many bytes fit in my buffer". These functions are not supposed to validate your input any more than are they supposed to check the validity of your pointers, intent, etc. If you're passing a pointer to something that is not a string (if you don't have a NUL terminator, your array by definition does not contain a string), you have a problem that you shouldn't hope string handling functions to magically cope with. > if you're absolutely paranoid about security, these functions not having both source size and target size parameters gives you a second way to go out-of-bounds. Target size ought to be enough for simple code dealing with actual strings. But if what you're dealing with isn't a string (and you need extra reassurances because you're not sure what it is), then how are you going to compute the source size? But if you can just compute the source size, then you can make sure what you have is a real string.
- byuu 13y ago
  > But this isn't the common scenario that strl* family of functions were designed for I will give you that. They definitely weren't designed for this scenario. But in practice, people tend to ignore the return value (which is using the functions incorrectly), and if you were used to strncpy (which has its own performance problems), you might wrongly assume that you could use these to pull substrings out as well. The regrettable part is that it's easy to combine overflow detection with substring copy functionality at the same time. > Sorry but that is just wrong. Sorry, but it is not wrong. See the actual implementation source: http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/strlcat.c?rev=1.13 http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/st... "Returns strlen(src) + MIN(siz, strlen(initial dst))." Even the official man page says the return value is confusing: "While this may seem somewhat confusing, it was done to make truncation detection simple." I mean no disrespect, but you're proving my point. Even most proponents of these functions do not understand all of the nuances in how they work. That's a real problem, as if you don't understand strlcat's return value, you can't use it safely. The functionality is too specialized to be general purpose functions. > It's not "requested length", it's "this many bytes fit in my buffer" Yes. Sorry if my wording was poor, I'm not really good at explaining things. > These functions are not supposed to validate your input any more than are they supposed to check the validity of your pointers, intent, etc. Exactly. They're useful functions for certain purposes. My only point is that they are by no means perfect. There are many use cases where they are not appropriate. Unfortunately people tend to either praise this family as the solution to C memory safety problems, or they dismiss them entirely. Absolutely use them when they're appropriate, but don't simply use them everywhere and think you're making your program safer. There's performance implications, out-of-bounds memory violations, security issues raised by truncating values without catching it, etc. These are all the programmer's fault for misusing the functions, of course, but safer and/or more versatile functions can be made.
  - clarry 13y ago
    > But in practice, people tend to ignore the return value (which is using the functions incorrectly) That might be true. It would be very interesting to see some statistics. However, with these functions, it is very easy to check whether the truncation detection is being done or not. Grep is mostly enough, but it can even be automated for most cases. Also, that is hardly an argument against these functions: any string utility function will inevitably have to deal with errors somehow (or worse, ignore them and continue!). > if you were used to strncpy (which has its own performance problems), you might wrongly assume that you could use these to pull substrings out as well. Yes, and sadly I've seen something like that (and worse) in the wild. These are easy to spot though. > Sorry, but it is not wrong. > > See the actual implementation source: http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/st.. http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/st.... > > "Returns strlen(src) + MIN(siz, strlen(initial dst))." Interesting. You're right about the code and its return value. However, the current manual documents it a little bit differently; the weird return value for the case when the initial destination string is not a string is never mentioned (but the fact that NUL-termination won't occur in this case is there). So in practice you can forget the MIN() and say it is strlen(dst) + strlen(src). From my perspective it makes all the sense, because, going back to what I've argued before, a string handling function should at least be allowed to assume you're giving it strings to work with; it's not there to validate your input or check pointers, etc. > Even the official man page says the return value is confusing: "While this may seem somewhat confusing, it was done to make truncation detection simple." You quoted a line that is not describing the corner-case return value mentioned above. It is describing the fact that the return value is (assuming actual strings are processed!) the total length of the string that would result from the operation if the destination buffer was large enough to hold it without truncation. There is nothing confusing about that. Perhaps the author initially thought it would be confusing for people who expect it to resemble functions such as read() that return the number of characters actually given to you. But there is nothing to be confused for, and the official man page has been simplified to reflect that: commit: http://marc.info/?l=openbsd-cvs&m=133338810915771&w=2 current version: http://mdoc.su/o/strlcat > I mean no disrespect, but you're proving my point. Even most proponents of these functions do not understand all of the nuances in how they work. That's a real problem, as if you don't understand strlcat's return value, you can't use it safely. The correct way to detect truncation with strlcpy and strlcat has always been exactly the same, and documented: if (strlcat(dst, src, dstsize) >= dstsize) { /* truncated */ } The fact that I wasn't aware of a corner-case scenario where dst does not contain a string to begin with has in no way impeded my ability to type the above line correctly. The branch will still be taken if the string would be too large for the buffer, but your code is broken and strlcat cannot fix it for you. > The functionality is too specialized to be general purpose functions Yes, they perform a very specific function; they are not Perl. Yet these are "general purpose" enough that the less secure variants (and not only these as of C11) were introduced in the C standard, and are used everywhere. These functions are convenient, available everywhere, and recognized by any C programmer, and that is why they are used. The OpenBSD functions came to be because of the widespread (mis)use of the C functions, to make it easier to do it right. > Exactly. They're useful functions for certain purposes. My only point is that they are by no means perfect. There are many use cases where they are not appropriate. Unfortunately people tend to either praise this family as the solution to C memory safety problems, or they dismiss them entirely. Good to know we are in agreement. :-) Perhaps I interpreted you too strongly when you said one may as well use another implementation; to me that sounded like a suggestion to dismiss these entirely. > These are all the programmer's fault for misusing the functions, of course, but safer and/or more versatile functions can be made. I would be interested in a concrete example. More versatile? Yes, that is easy. Safer? Well I'm not so sure about that. Perhaps by trading some correctness. Perhaps in some non-critical user-land application where unbounded allocations are OK and aborting on memory exhaustion is "safe". Simpler and safer? Almost definitely not, but please show me I'm wrong :-)

by byuu 13y ago

clarry 13y ago

> strl(cpy|cat)() is quite dangerous to just use blindly Very true; people have caused serious issues by blindly doing things (supposedly making things secure or else), without bothering to understand what they're doing. However, for the specific (and reasonably common) scenario of dealing with proper C strings, the strl* functions are very simple and easy to use correctly, and their use is easy to audit. > So say you have a big 20MB XML document, and as you are scanning along, you use strlcpy() to store a node name or value into another string. [..] If you keep doing this over the entire document, your parser will end up with quadratic overhead. Right. It is going to be ridiculously slow (and that should be obvious). But this isn't the common scenario that strl* family of functions were designed for (and which a lot of people get wrong with standard C string functions). But chances are in code like this C strings' shortcomings would be problematic anyway. > strlcat() [..] its return value is downright cryptic: it is strlen(source) + min(length, strlen(target)). Sorry but that is just wrong. > When it tries to compute strlen(), ignoring your requested length parameter, this means it will access out-of-bounds memory if that string is not properly null-terminated. It's not "requested length", it's "this many bytes fit in my buffer". These functions are not supposed to validate your input any more than are they supposed to check the validity of your pointers, intent, etc. If you're passing a pointer to something that is not a string (if you don't have a NUL terminator, your array by definition does not contain a string), you have a problem that you shouldn't hope string handling functions to magically cope with. > if you're absolutely paranoid about security, these functions not having both source size and target size parameters gives you a second way to go out-of-bounds. Target size ought to be enough for simple code dealing with actual strings. But if what you're dealing with isn't a string (and you need extra reassurances because you're not sure what it is), then how are you going to compute the source size? But if you can just compute the source size, then you can make sure what you have is a real string.

byuu 13y ago
> But this isn't the common scenario that strl* family of functions were designed for I will give you that. They definitely weren't designed for this scenario. But in practice, people tend to ignore the return value (which is using the functions incorrectly), and if you were used to strncpy (which has its own performance problems), you might wrongly assume that you could use these to pull substrings out as well. The regrettable part is that it's easy to combine overflow detection with substring copy functionality at the same time. > Sorry but that is just wrong. Sorry, but it is not wrong. See the actual implementation source: http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/strlcat.c?rev=1.13 http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/st... "Returns strlen(src) + MIN(siz, strlen(initial dst))." Even the official man page says the return value is confusing: "While this may seem somewhat confusing, it was done to make truncation detection simple." I mean no disrespect, but you're proving my point. Even most proponents of these functions do not understand all of the nuances in how they work. That's a real problem, as if you don't understand strlcat's return value, you can't use it safely. The functionality is too specialized to be general purpose functions. > It's not "requested length", it's "this many bytes fit in my buffer" Yes. Sorry if my wording was poor, I'm not really good at explaining things. > These functions are not supposed to validate your input any more than are they supposed to check the validity of your pointers, intent, etc. Exactly. They're useful functions for certain purposes. My only point is that they are by no means perfect. There are many use cases where they are not appropriate. Unfortunately people tend to either praise this family as the solution to C memory safety problems, or they dismiss them entirely. Absolutely use them when they're appropriate, but don't simply use them everywhere and think you're making your program safer. There's performance implications, out-of-bounds memory violations, security issues raised by truncating values without catching it, etc. These are all the programmer's fault for misusing the functions, of course, but safer and/or more versatile functions can be made.
- clarry 13y ago
  > But in practice, people tend to ignore the return value (which is using the functions incorrectly) That might be true. It would be very interesting to see some statistics. However, with these functions, it is very easy to check whether the truncation detection is being done or not. Grep is mostly enough, but it can even be automated for most cases. Also, that is hardly an argument against these functions: any string utility function will inevitably have to deal with errors somehow (or worse, ignore them and continue!). > if you were used to strncpy (which has its own performance problems), you might wrongly assume that you could use these to pull substrings out as well. Yes, and sadly I've seen something like that (and worse) in the wild. These are easy to spot though. > Sorry, but it is not wrong. > > See the actual implementation source: http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/st.. http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/st.... > > "Returns strlen(src) + MIN(siz, strlen(initial dst))." Interesting. You're right about the code and its return value. However, the current manual documents it a little bit differently; the weird return value for the case when the initial destination string is not a string is never mentioned (but the fact that NUL-termination won't occur in this case is there). So in practice you can forget the MIN() and say it is strlen(dst) + strlen(src). From my perspective it makes all the sense, because, going back to what I've argued before, a string handling function should at least be allowed to assume you're giving it strings to work with; it's not there to validate your input or check pointers, etc. > Even the official man page says the return value is confusing: "While this may seem somewhat confusing, it was done to make truncation detection simple." You quoted a line that is not describing the corner-case return value mentioned above. It is describing the fact that the return value is (assuming actual strings are processed!) the total length of the string that would result from the operation if the destination buffer was large enough to hold it without truncation. There is nothing confusing about that. Perhaps the author initially thought it would be confusing for people who expect it to resemble functions such as read() that return the number of characters actually given to you. But there is nothing to be confused for, and the official man page has been simplified to reflect that: commit: http://marc.info/?l=openbsd-cvs&m=133338810915771&w=2 current version: http://mdoc.su/o/strlcat > I mean no disrespect, but you're proving my point. Even most proponents of these functions do not understand all of the nuances in how they work. That's a real problem, as if you don't understand strlcat's return value, you can't use it safely. The correct way to detect truncation with strlcpy and strlcat has always been exactly the same, and documented: if (strlcat(dst, src, dstsize) >= dstsize) { /* truncated */ } The fact that I wasn't aware of a corner-case scenario where dst does not contain a string to begin with has in no way impeded my ability to type the above line correctly. The branch will still be taken if the string would be too large for the buffer, but your code is broken and strlcat cannot fix it for you. > The functionality is too specialized to be general purpose functions Yes, they perform a very specific function; they are not Perl. Yet these are "general purpose" enough that the less secure variants (and not only these as of C11) were introduced in the C standard, and are used everywhere. These functions are convenient, available everywhere, and recognized by any C programmer, and that is why they are used. The OpenBSD functions came to be because of the widespread (mis)use of the C functions, to make it easier to do it right. > Exactly. They're useful functions for certain purposes. My only point is that they are by no means perfect. There are many use cases where they are not appropriate. Unfortunately people tend to either praise this family as the solution to C memory safety problems, or they dismiss them entirely. Good to know we are in agreement. :-) Perhaps I interpreted you too strongly when you said one may as well use another implementation; to me that sounded like a suggestion to dismiss these entirely. > These are all the programmer's fault for misusing the functions, of course, but safer and/or more versatile functions can be made. I would be interested in a concrete example. More versatile? Yes, that is easy. Safer? Well I'm not so sure about that. Perhaps by trading some correctness. Perhaps in some non-critical user-land application where unbounded allocations are OK and aborting on memory exhaustion is "safe". Simpler and safer? Almost definitely not, but please show me I'm wrong :-)