4 ms·

strncpy won't always write a trailing nul byte, causing out of bounds reads elsewhere. It's a nasty little fellow. See the warning at https://linux.die.net/man/

by i80and 3y ago

strncpy won't always write a trailing nul byte, causing out of bounds reads elsewhere. It's a nasty little fellow. See the warning at https://linux.die.net/man/3/strncpy https://linux.die.net/man/3/strncpy strlcpy() is better and what most people think strncpy() is, but still results in truncated strings if not used carefully which can also lead to big problems.

sirwhinesalot 3y ago
Speaking of strlcpy, Linus has some colorful opinions on it: > Note that we have so few 'strlcpy()' calls that we really should remove that horrid horrid interface. It's a buggy piece of sh*t. 'strlcpy()' is fundamentally unsafe BY DESIGN if you don't trust the source string - which is one of the alleged reasons to use it. --Linus Maybe strscpy is finally the one true fixed design to fix them all. Personally I think the whole exercise is one of unbeliavable stupidity when the real solution is obvious: using proper string buffer types with length and capacity for any sort of string manipulation.
- jjav 3y ago
  > the real solution is obvious If it were obvious it would have been done already. Witness the many variants that try to make it better but don't. > using proper string buffer types with length and capacity Which you then can't pass to any other library. String management is very easy to solve within the boundaries of your own code. But you'll need to interact with existing code as well.
  - sirwhinesalot 3y ago
    > If it were obvious it would have been done already. Witness the many variants that try to make it better but don't. Every other language with mutable strings, including C++, does it like that. It is obvious. The reason it is not done in C is not ignorance, it is laziness. > Which you then can't pass to any other library. String management is very easy to solve within the boundaries of your own code. But you'll need to interact with existing code as well. Ignoring the also obvious solution of just keeping a null terminator around (see: C++ std::string), you should only worry about it at the boundary with the other library. Same as converting from utf-8 to utf-16 to talk to the Windows API for example.
    - jjav 3y ago
      > The reason it is not done in C is not ignorance, it is laziness. Of course not. C has been around since the dawn of UNIX and the majority of important libraries at the OS level are written in it. Compatibility with such a vast amount of code is a lot more important than anything else. If it were so easy why do you think nobody has done it? > Ignoring the also obvious solution of just keeping a null terminator around That's not very useful for the general case. If your code relies on the extra metadata (length, size) being correct and you're passing that null-terminated buffer around to libraries outside your code, it won't be correct since nothing else is aware of it.
      - sirwhinesalot 3y ago
        > If it were so easy why do you think nobody has done it? People have done it, there are plenty strbuf implementations to go around. Even the kernel has seq_buf. How you handle string manipulation internally in your codebase does not matter for compatibility with existing libraries. > That's not very useful for the general case. If your code relies on the extra metadata (length, size) being correct and you're passing that null-terminated buffer around to libraries outside your code, it won't be correct since nothing else is aware of it. You can safely pass the char* buffer inside a std::string to any C library with no conversion. You're making up issues in your head. Don't excuse incompetence.
        jjav 3y ago
        > People have done it, there are plenty strbuf implementations to go around. Precisely! Why plenty and why is none of them the standard in C?
        sirwhinesalot 3y ago
        The TL;DR on that is basically "lazy, security unconscious assholes keep shutting it down". Dennies Ritchie strongly suggested C should add fat pointers all the way back in 1990. Other people have pointed out the issues with zero terminated strings and arrays decaying into pointers (and the ways to deal with them even with backwards compatibility constraints) for years. One of the most prominent was Walter Bright's article on "C's Biggest Mistake" back in 2009 and he was a C/C++ commercial compiler developer. There is no excuse.
  - jandrese 3y ago
    For me the "real" solution looks something like this: ssize_t strxcpy(char* restrict dst, const char* restrict src, ssize_t len) Strxcpy copies the string from src to dst. The len parameter is the number of bytes available in the dst buffer. The dst buffer is always terminated with a null byte, so the maximum length of string that can be copied into it is len - 1. strxcpy returns the number of characters copied on success, but can return the following negative values: E_INVALID_PARAMETER: Ether dst or src are NULL or len < 1, no data was copied W_TRUNCATED: len - 1 bytes were copied but more characters were available in src. strxcat would work similarly. I have not decided if the return value should include the terminating null or not.
    - jjav 3y ago
      How is this useful though? I mean yes, it is useful in avoiding the buffer overruns. But that's not the only consideration, you also want code that handles data correctly. This just truncates at buffer size so data is lost. So, if you want the code to work correctly, you need to either check the return code and reallocate dst and call the copy again. But if you're going to do that might as well check src len and allocate dst correctly before calling it so it never fails. But if you're already doing that, you can call strcpy just fine and never have a problem.
      - jandrese 3y ago
        Sometimes truncation is fine or at least can be managed. Yes, strdup() is a better choice in a lot of situations, but depending on how your data is structured it may not be the correct option. I would say my version is useful in any situation where you were previously using strncpy/cat or strlcpy/cat.
- raverbashing 3y ago
  Wow yeah this seems to summarize well the usual api flakiness and just shuffling of C It seems people come with "one more improvement" that's broken in one way or the other
jandrese 3y ago
The problem with strlcpy is the return value. You can be burned badly if you are using it to for example pull out a fixed chunk of string from a 10TB memory mapped file, especially if you're pulling out all of the 32 byte chunks from that huge file and you just wanted a function to stick the trailing 0 on the string and handle short reads gracefully. It's even worse if you are using it because you don't fully trust the input string to be null terminated. Maybe you have reasons to be believe that it will be at least as long as you need, but can't trust that it is a real string. As a function that was theoretically written as "fix" for strncpy it is worse in some fundamental ways. At least strncpy is easy enough to make safe by always over-allocating your buffer by 1 byte and stuffing a 0 in the last byte.
Borg3 3y ago
#define strncpyz(d,s,l) *(strncpy(d,s,l)+(l))=0 Of course this one is unsafe for macro expansion. But well, its C :)
- teo_zero 3y ago
  I'd rather put the final nul at d+l-1 than at d+l, so that l can be the size of d, not "one more than the size of d": strncpyz(buf,src,sizeof buf);
kevin_thibedeau 3y ago
strncpy() also zero pads the entire buffer. If it's significantly larger than the copied string you're wasting cycles on pointless move operations for normal, low-security string handling. This behavior is for filling in fixed length fields in data structures. It isn't suitable for general purpose string processing.

by i80and 3y ago

sirwhinesalot 3y ago

Speaking of strlcpy, Linus has some colorful opinions on it: > Note that we have so few 'strlcpy()' calls that we really should remove that horrid horrid interface. It's a buggy piece of sh*t. 'strlcpy()' is fundamentally unsafe BY DESIGN if you don't trust the source string - which is one of the alleged reasons to use it. --Linus Maybe strscpy is finally the one true fixed design to fix them all. Personally I think the whole exercise is one of unbeliavable stupidity when the real solution is obvious: using proper string buffer types with length and capacity for any sort of string manipulation.

jjav 3y ago
> the real solution is obvious If it were obvious it would have been done already. Witness the many variants that try to make it better but don't. > using proper string buffer types with length and capacity Which you then can't pass to any other library. String management is very easy to solve within the boundaries of your own code. But you'll need to interact with existing code as well.
- sirwhinesalot 3y ago
  > If it were obvious it would have been done already. Witness the many variants that try to make it better but don't. Every other language with mutable strings, including C++, does it like that. It is obvious. The reason it is not done in C is not ignorance, it is laziness. > Which you then can't pass to any other library. String management is very easy to solve within the boundaries of your own code. But you'll need to interact with existing code as well. Ignoring the also obvious solution of just keeping a null terminator around (see: C++ std::string), you should only worry about it at the boundary with the other library. Same as converting from utf-8 to utf-16 to talk to the Windows API for example.
  - jjav 3y ago
    > The reason it is not done in C is not ignorance, it is laziness. Of course not. C has been around since the dawn of UNIX and the majority of important libraries at the OS level are written in it. Compatibility with such a vast amount of code is a lot more important than anything else. If it were so easy why do you think nobody has done it? > Ignoring the also obvious solution of just keeping a null terminator around That's not very useful for the general case. If your code relies on the extra metadata (length, size) being correct and you're passing that null-terminated buffer around to libraries outside your code, it won't be correct since nothing else is aware of it.
    - sirwhinesalot 3y ago
      > If it were so easy why do you think nobody has done it? People have done it, there are plenty strbuf implementations to go around. Even the kernel has seq_buf. How you handle string manipulation internally in your codebase does not matter for compatibility with existing libraries. > That's not very useful for the general case. If your code relies on the extra metadata (length, size) being correct and you're passing that null-terminated buffer around to libraries outside your code, it won't be correct since nothing else is aware of it. You can safely pass the char* buffer inside a std::string to any C library with no conversion. You're making up issues in your head. Don't excuse incompetence.
      - jjav 3y ago
        > People have done it, there are plenty strbuf implementations to go around. Precisely! Why plenty and why is none of them the standard in C?
        sirwhinesalot 3y ago
        The TL;DR on that is basically "lazy, security unconscious assholes keep shutting it down". Dennies Ritchie strongly suggested C should add fat pointers all the way back in 1990. Other people have pointed out the issues with zero terminated strings and arrays decaying into pointers (and the ways to deal with them even with backwards compatibility constraints) for years. One of the most prominent was Walter Bright's article on "C's Biggest Mistake" back in 2009 and he was a C/C++ commercial compiler developer. There is no excuse.
- jandrese 3y ago
  For me the "real" solution looks something like this: ssize_t strxcpy(char* restrict dst, const char* restrict src, ssize_t len) Strxcpy copies the string from src to dst. The len parameter is the number of bytes available in the dst buffer. The dst buffer is always terminated with a null byte, so the maximum length of string that can be copied into it is len - 1. strxcpy returns the number of characters copied on success, but can return the following negative values: E_INVALID_PARAMETER: Ether dst or src are NULL or len < 1, no data was copied W_TRUNCATED: len - 1 bytes were copied but more characters were available in src. strxcat would work similarly. I have not decided if the return value should include the terminating null or not.
  - jjav 3y ago
    How is this useful though? I mean yes, it is useful in avoiding the buffer overruns. But that's not the only consideration, you also want code that handles data correctly. This just truncates at buffer size so data is lost. So, if you want the code to work correctly, you need to either check the return code and reallocate dst and call the copy again. But if you're going to do that might as well check src len and allocate dst correctly before calling it so it never fails. But if you're already doing that, you can call strcpy just fine and never have a problem.
    - jandrese 3y ago
      Sometimes truncation is fine or at least can be managed. Yes, strdup() is a better choice in a lot of situations, but depending on how your data is structured it may not be the correct option. I would say my version is useful in any situation where you were previously using strncpy/cat or strlcpy/cat.
raverbashing 3y ago
Wow yeah this seems to summarize well the usual api flakiness and just shuffling of C It seems people come with "one more improvement" that's broken in one way or the other

jandrese 3y ago

The problem with strlcpy is the return value. You can be burned badly if you are using it to for example pull out a fixed chunk of string from a 10TB memory mapped file, especially if you're pulling out all of the 32 byte chunks from that huge file and you just wanted a function to stick the trailing 0 on the string and handle short reads gracefully. It's even worse if you are using it because you don't fully trust the input string to be null terminated. Maybe you have reasons to be believe that it will be at least as long as you need, but can't trust that it is a real string. As a function that was theoretically written as "fix" for strncpy it is worse in some fundamental ways. At least strncpy is easy enough to make safe by always over-allocating your buffer by 1 byte and stuffing a 0 in the last byte.

Borg3 3y ago

#define strncpyz(d,s,l) *(strncpy(d,s,l)+(l))=0 Of course this one is unsafe for macro expansion. But well, its C :)

teo_zero 3y ago
I'd rather put the final nul at d+l-1 than at d+l, so that l can be the size of d, not "one more than the size of d": strncpyz(buf,src,sizeof buf);

kevin_thibedeau 3y ago

strncpy() also zero pads the entire buffer. If it's significantly larger than the copied string you're wasting cycles on pointless move operations for normal, low-security string handling. This behavior is for filling in fixed length fields in data structures. It isn't suitable for general purpose string processing.