Is it possible to exploit a vulnerable function if its input is safe?

Question

Assume that I have a code having buffer overflow vulnerability as following

int func(const char *str){
    char buffer[100];
    unsigned short len = strlen(str);

    if(len >= 100){
        return -1;
    }
    strncpy(buffer,str,strlen(str));
    return 0; 
}

(taken from this question)

Is there a way to exploit this vulnerability if its getting input from another function (not user input) and the length of str is always less than 100?

For example

int main() {
    int user_input;
    if (cin >> user_input) {
        if(user_input == 1)
          func("aaaa");
        else 
          func("bbbb");
    }
}

Assume there is no other vulnerability in the code.

Just a hypothetical question, any ideas?

If the length of str is always less than 100, there's no vulnerability. — Anton Savin, Apr 30 '15 at 07:46
@AntonSavin So the codes that are not getting user input is always safe if they are running? — smttsp, Apr 30 '15 at 07:53
I don't see the reason for `if(len >= 100)` if you are sure that `len` will always be less than 100 — Spikatrix, Apr 30 '15 at 07:53
@smttsp Have you read the answers to the question you linked? The vulnerability occurs if `strlen(str) > 65535`, wherever `str` comes from doesn't matter — Anton Savin, Apr 30 '15 at 07:56
@AntonSavin, yes I have read the question, but in that question, `strlen(str)` can be greater than `65535`. To be able to guarantee that `strlen(str) < 65535`, I said another function. it can be trusted user if you want that always enters a short string. That is not the deal. I'm searching a way if user is not giving an input, is there anything to do? — smttsp, Apr 30 '15 at 08:03
Your `main` has a serious memory corruption. You must not read into the bytes of a string literal, the conversion of which to non-`const` `char *` is deprecated by the way and should issue a compiler warning. And even if you were allowed to write into `user_input`'s bytes, the array would be too short to hold even the string `"1"`. Also, comparing `char` buffers with `==` will likely not do what you expect. — 5gon12eder, Apr 30 '15 at 08:09
@5gon12eder: I was expecting to have some problems about `char*`, so I wrote assume there is no other vulnerability. But thanks for showing it. Could you please correct the memory corruption in `main`? — smttsp, Apr 30 '15 at 08:14
@smttsp: sorry I deleted my answer. I assumed that the question was more general and failed to spot the "obvious" issues. — stefaanv, Apr 30 '15 at 08:14
@stefaanv It was fine actually, I was about to upvote as the `EDIT` part was helpful. So it would be better if you can undelete — smttsp, Apr 30 '15 at 08:16
The strlen(str) as parameter of strncpy is a serious bug, always creating a non-nul terminated string because strncpy thinks the destination buffer can only contain the string without the terminating nul, which could be exploited if somehow this code would work normally. The parameter should be sizeof(buffer). — stefaanv, Apr 30 '15 at 08:40
Ok, I did it. Please consider editing in @stefaanv's point too if it is not what you've meant to ask about. Since your toy code never actually uses the buffer where you copied the bytes to, the bug will not show up in this particular code. — 5gon12eder, Apr 30 '15 at 08:43

score 2 · Accepted Answer · answered May 28 '15 at 12:47

In short, there is no vulnerability. Every input sanitized = no vulnerability.

But that doesn't mean you should leave it unfixed. While there is no physical vulnerability, there is a lot of potential for a vulnerability. Now you don't pass anything longer than 100 characters. But what about a few months from now on? Will you remember that you can only pass input shorter than 100 characters? I don't think so.

You can fix it by:

choosing to hold strlen in size_t (but this won't circumvent buffer overflow if variable is longer than 4GB)
using a dynamically allocated buffer and checking if you managed to malloc it successfully
using strnlen together with sizeof(buffer) rather than strlen
passing len as a second parameter (probably annoying)

Using strncpy(a, b, strlen(b)) is the same as using strcpy(a,b). This is prevented to some extent with the check in the if instruction, but the choice unsigned short for its storage makes it worthless anyway. It's also better to use strncpy(a, b, len) to make it obvious that len does need to be there, in case the check gets refactored away.

Is it possible to exploit a vulnerable function if its input is safe?

1 Answers1