Possible "Integer Overflow or Wraparound" flaw in this C code

Question

Below I have extracted a piece of code from a project over which I ran a static analysis tool looking for security flaws. It flagged this code as being susceptible to an integer overflow/wraparound flaw as explained here:

https://cwe.mitre.org/data/definitions/190.html

Here is the relevant code:

#define STRMAX 16

void bar() {
    // ... struct malloc'd elsewhere     
    mystruct->string = malloc(STRMAX * sizeof(char));
    memset(mystruct->string, '\0', STRMAX);
    strncpy(mystruct->string,"H3C19H1E4XAA9MQ",STRMAX); // 15 character string

    mystruct->baz = malloc(3 * sizeof(char*));
    memset(mystruct->baz, '\0', 3); 

    if (strlen(mystruct->string) > 0) {
        strncpy(mystruct->baz,&(mystruct->string[0]),2);
    }

    mystruct->quux = malloc(19 * sizeof(char));
    memset(mystruct->quux, '\0', 19);
    for(int i = 0; i < 19; i++) {
        mystruct->quux[i] = 'a';
    }

    foo(struct);
}

void foo(Mystruct *struct) {

    // this was flagged
    size_t input_len = strlen(mystruct->baz) + strlen(mystruct->quux);
    // this one too but it flows from the first I think.
    char *input = malloc((input_len + 1) * sizeof(char));
    memset(input, '\0', input_len + 1);
    strncpy(input, mystruct->baz, strlen(mystruct->baz));
    strncat(input, mystruct->quux, strlen(mystruct->quux));
    // ...
}

So in other words, there are three members of a struct that are explicitly bounded in one function and then used to create a variable in another one based on the size of the struct's members.

The analyzer flagged the first line of foo in particular. My question is, did it correctly flag this? If so, what would be a simple way to mitigate this? I'm on a platform that doesn't have a BSD-style reallocate function by default.

PS: I realize that strncpy(foo, bar, strlen(bar)) is somewhat frivolous in memory terms.

Answer 1

1. size_t input_len = strlen(struct->baz) + strlen(struct->quux); can theoretically wraparound so the analyser is right. If strlen(struct->quux) is larger than SIZE_MAX - strlen(struct->baz) it will wraparound

2. If input_len == SIZE_MAX then input_len + 1 can wraparound as well.

Answer 2

When you add two string lengths, the result can exceed the maximum value of the size_t type. For instance, if SIZE_MAX were 100 (I know this is not realistic, it's just for example purposes), and your strings were 70 and 50 characters long, the total would wrap around to 20, which is not the correct result.

This is rarely a real concern, since the actual maximum is usually very large (the C standard allows it to be as low as 65535, but you're only likely to run into this on microcontrollers), and in practice strings are not even close to the limit, so most programmers would just ignore this problem. Protecting against this overflow gets complicated, although it's possible if you really need to.