Categories
c security strlcpy strncpy

Why are strlcpy and strlcat considered insecure?

84

I understand that strlcpy and strlcat were designed as secure replacements for strncpy and strncat. However, some people are still of the opinion that they are insecure, and simply cause a different type of problem.

Can someone give an example of how using strlcpy or strlcat (i.e. a function that always null terminates its strings) can lead to security problems?

Ulrich Drepper and James Antill state this is true, but never provide examples or clarify this point.

    32

    Ulrich’s criticism is based on the idea that a string truncation that is not detected by the program can lead to security issues, through incorrect logic. Therefore, to be secure, you need to check for truncation. To do this for a string concatenation means that you are doing a check along the lines of this:

    if (destlen + sourcelen > dest_maxlen)
    {
        /* Bug out */
    }
    

    Now, strlcat does effectively do this check, if the programmer remembers to check the result – so you can use it safely:

    if (strlcat(dest, source, dest_bufferlen) >= dest_bufferlen)
    {
        /* Bug out */
    }
    

    Ulrich’s point is that since you have to have destlen and sourcelen around (or recalculate them, which is what strlcat effectively does), you might as well just use the more efficient memcpy anyway:

    if (destlen + sourcelen > dest_maxlen)
    {
        goto error_out;
    }
    memcpy(dest + destlen, source, sourcelen + 1);
    destlen += sourcelen;
    

    (In the above code, dest_maxlen is the maximum length of the string that can be stored in dest – one less than the size of the dest buffer. dest_bufferlen is the full size of the dest buffer).

    6

    • 14

      The readability of Drepper’s code is bad. With strlcpy (or any str function) I know directly that I’m copying a 0 terminated C string. With memcpy it can be any type of memory and I have a supplemental dimension to check when trying to understand the code. I had a legacy app to debug where everything was done with memcpy, it was a real PITA to correct. After porting to dedicated String function it is much easier to read (and faster because a lot of unnecessary strlen could be removed).

      Nov 26, 2010 at 15:51


    • 3

      @domen: Because the size to copy is already known, so memcpy() is sufficient (and is potentially more efficient than strcpy()).

      – caf

      Nov 19, 2014 at 4:22

    • 1

      Well, it’s confusing to have it in string operations. And as far as I know efficiency depends on implementation and is not standardized.

      – domen

      Nov 19, 2014 at 9:07

    • 8

      @domen: memcpy() is a string operation – it’s declared in <string.h>, after all.

      – caf

      Nov 20, 2014 at 2:58

    • 2

      @domen I agree that there is potential for confusion, but the reality is that working with C strings is pretty much working with raw memory anyway. Arguably, we would all be better off if people just stopped thinking of C as having “strings” (as distinct from any other contiguous memory chunks) at all.

      – mtraceur

      Feb 2, 2018 at 3:27

    22

    When people say, “strcpy() is dangerous, use strncpy() instead” (or similar statements about strcat() etc., but I am going to use strcpy() here as my focus), they mean that there is no bounds checking in strcpy(). Thus, an overly long string will result in buffer overruns. They are correct. Using strncpy() in this case will prevent buffer overruns.

    I feel that strncpy() really doesn’t fix bugs: it solves a problem that can be easily avoided by a good programmer.

    As a C programmer, you must know the destination size before you are trying to copy strings. That is the assumption in strncpy() and strlcpy()‘s last parameters too: you supply that size to them. You can also know the source size before you copy strings. Then, if the destination is not big enough, don’t call strcpy(). Either reallocate the buffer, or do something else.

    Why do I not like strncpy()?

    • strncpy() is a bad solution in most cases: your string is going to be truncated without any notice—I would rather write extra code to figure this out myself and then take the course of action that I want to take, rather than let some function decide for me about what to do.
    • strncpy() is very inefficient. It writes to every byte in the destination buffer. You don’t need those thousands of '\0' at the end of your destination.
    • It doesn’t write a terminating '\0' if the destination is not big enough. So, you must do so yourself anyway. The complexity of doing this is not worth the trouble.

    Now, we come to strlcpy(). The changes from strncpy() make it better, but I am not sure if the specific behavior of strl* warrants their existence: they are far too specific. You still have to know the destination size. It is more efficient than strncpy() because it doesn’t necessarily write to every byte in the destination. But it solves a problem that can be solved by doing: *((char *)mempcpy(dst, src, n)) = 0;.

    I don’t think anyone says that strlcpy() or strlcat() can lead to security issues, what they (and I) are saying that they can result in bugs, for example, when you expect the complete string to be written instead of a part of it.

    The main issue here is: how many bytes to copy? The programmer must know this and if he doesn’t, strncpy() or strlcpy() won’t save him.

    strlcpy() and strlcat() are not standard, neither ISO C nor POSIX. So, their use in portable programs is impossible. In fact, strlcat() has two different variants: the Solaris implementation is different from the others for edge cases involving length 0. This makes it even less useful than otherwise.

    7

    • 3

      strlcpy is faster than memcpy on many architectures, especially if the memcpy copies unnecessary trailing data. strlcpy also returns how much data you missed which might allow you to recover faster and with less code.

      – user14554

      Jan 22, 2010 at 5:15

    • 1

      @jbcreix: my point is that there should be no data to miss, and n in my memcpy call is the exact number of bytes to be written, so the efficiency isn’t that much of a problem either.

      Jan 22, 2010 at 11:06

    • 5

      And how do you get that n? The only n you can know in advance is the buffer size. Of course if you suggest re implementing strlcpy each time you need it using memcpy and strlen that is okay too, but then why stopping at strlcpy, you don’t need a memcpy function either, you can copy the bytes one by one. The reference implementation only loops through the data once in the normal case and that is better for most architectures. But even if the best implementation used strlen + memcpy, that is still no reason to not having to re-implement a secure strcpy again and again.

      – user14554

      Jan 22, 2010 at 11:30

    • 1

      Alok, with strlcpy you sweep over the input string only once in the good case (which should be the majority) and twice in the bad case, with your strlen+memcpy you go over it twice always. If it makes a difference in practice is another story.

      Nov 26, 2010 at 15:44

    • 4

      “*((char *)mempcpy(dst, src, n)) = 0;” – Correct – Yes, Obviously Correct, No. Fails code review……

      – mattnz

      Jul 9, 2013 at 1:48