It's been too long since I've done any assembly programming, but this really doesn't appear to have good documentation for maintainability; or anything resembling sufficient documentation at all.
Not really. The strlcat function is safer because it always NUL terminates the destination buffer, and therefore makes bugs in user code less likely. The flaw here is just an implementation defect.
Also note that SIZE_MAX is an unreasonable input, one would even say it violates preconditions.
Commonly, a policy declared that all use of `strcat` must be replaced with `strncat` and at one or more locations in a codebase an actual size bound could not be ascertained without more engineering effort than management was willing to spend.
No use case I can think of, but a credible scenario is an attacker somehow being able to manipulate the size parameter, due to another vulnerability, and then being able to further exploit the overflow.
Quite a few recent practical attacks chained what looked like mostly harmless vulnerabilities taken individually.
Imagine you have a user configurable buffer size. Then imagine that the user wants to throw as much ram as it takes at the problem and sets the maximum size to that value. It happens more often than you'd think.
I would argue that copying more bytes (as demonstrated) is correct behaviour:
(1.) The function is supposed to append a string to another, not paste a byte range into another one, leaving extra bytes at the end untouched. Thus the functional result in the test code is correct (i.e. contains the concatenated string with for the usual definition of NUL-terminated strings.)
(2.) The strncat man page (and similarly the C99 standard) says: "[...] the size of dest must be at least strlen(dest)+n+1." It does not specify what happens to unused bytes at the end. It would be incorrect if the function would ever touch more than `strlen(dest)+n+1` bytes in dest, but it doesn't. Supplying a proper value for n in the code (e.g. `strlen(s2)`) shows that it wouldn't overrun the buffer.
Thus supplying SIZE_MAX for `n` would be a bad idea (because the function could overwrite random memory locations) and would probably be incorrect in most cases (e.g. on amd64 with SIZE_MAX=2^64-1 it would only be correct for the null pointer.)
Copying less would obviously be incorrect. I couldn't reproduce that though. (The more case even appears on Ubuntu with eglibc on both x86 and amd64.)
Also, this is probably still a bug and not the anticipated behaviour. (As in: whoever wrote that didn't think of those cases and the overflow happens by mistake.)
> The strncat man page (and similarly the C99 standard) says: "[...] the size of dest must be at least strlen(dest)+n+1."
No, the standard does not say that. It (both C99 and C11) says, verbatim: "the maximum number of characters that can end up in the array pointed to by s1 is strlen(s1)+n+1." If the manpage on your system says that, that's a fairly serious deficiency in the manpage.
The standard also says "characters that follow it [a null character] are not appended", so copying extra bytes is explicitly not correct.
[+] [-] mjevans|10 years ago|reply
https://sourceware.org/glibc/wiki/GlibcGit
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/x86...
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/x86...
It's been too long since I've done any assembly programming, but this really doesn't appear to have good documentation for maintainability; or anything resembling sufficient documentation at all.
[+] [-] Kristine1975|10 years ago|reply
[+] [-] nomadlogic|10 years ago|reply
[+] [-] dietrichepp|10 years ago|reply
Also note that SIZE_MAX is an unreasonable input, one would even say it violates preconditions.
[+] [-] chanzmao|10 years ago|reply
[+] [-] martin_|10 years ago|reply
[+] [-] stephencanon|10 years ago|reply
[+] [-] brohee|10 years ago|reply
Quite a few recent practical attacks chained what looked like mostly harmless vulnerabilities taken individually.
[+] [-] mjevans|10 years ago|reply
[+] [-] nly|10 years ago|reply
[+] [-] xroche|10 years ago|reply
[+] [-] __michaelg|10 years ago|reply
(1.) The function is supposed to append a string to another, not paste a byte range into another one, leaving extra bytes at the end untouched. Thus the functional result in the test code is correct (i.e. contains the concatenated string with for the usual definition of NUL-terminated strings.)
(2.) The strncat man page (and similarly the C99 standard) says: "[...] the size of dest must be at least strlen(dest)+n+1." It does not specify what happens to unused bytes at the end. It would be incorrect if the function would ever touch more than `strlen(dest)+n+1` bytes in dest, but it doesn't. Supplying a proper value for n in the code (e.g. `strlen(s2)`) shows that it wouldn't overrun the buffer.
Thus supplying SIZE_MAX for `n` would be a bad idea (because the function could overwrite random memory locations) and would probably be incorrect in most cases (e.g. on amd64 with SIZE_MAX=2^64-1 it would only be correct for the null pointer.)
Copying less would obviously be incorrect. I couldn't reproduce that though. (The more case even appears on Ubuntu with eglibc on both x86 and amd64.)
Also, this is probably still a bug and not the anticipated behaviour. (As in: whoever wrote that didn't think of those cases and the overflow happens by mistake.)
[+] [-] stephencanon|10 years ago|reply
No, the standard does not say that. It (both C99 and C11) says, verbatim: "the maximum number of characters that can end up in the array pointed to by s1 is strlen(s1)+n+1." If the manpage on your system says that, that's a fairly serious deficiency in the manpage.
The standard also says "characters that follow it [a null character] are not appended", so copying extra bytes is explicitly not correct.
[+] [-] ambrop7|10 years ago|reply
[+] [-] halayli|10 years ago|reply