My code runs fine with test A or test B commented out, but both are run the program crashes

Question

#include <stdio.h>
#include <stdlib.h>
#include <inttypes.h>
#include <math.h>

void ip2array(const char* ip_str, uint8_t* ip_int) {
    int i = 0;
    char* p = (char*)ip_str;
    while (*p != '\0') {
        ip_int[i++] = (uint8_t)strtoul(p, &p, 10);
        p++;
    }
}

uint32_t ips_between (const char* start, const char* end) {
    uint8_t ip[2][4];
    ip2array(start, ip[0]);
    ip2array(end, ip[1]);

    uint32_t total = 0;
    for (int i = 0; i < 4; ++i)
        total += (ip[1][3 - i] - ip[0][3 - i]) * (uint32_t)pow(2, i * 8);

    return total;
}

int main() {
    printf("%u\n", ips_between("10.0.0.0", "10.0.0.50")); // test A
    printf("%u\n", ips_between("20.0.0.10", "20.0.1.0")); // test B
}

If I run the program with only test A or test B, the correct result is given. If the tests are run sequentially (as above) the program crashes: Process finished with exit code 134 (interrupted by signal 6: SIGABRT).

I have removed all dynamically allocated memory and double checked all the loops. When using a debugger, the program seems to crash just as it returns the total to main. I'm fairly sure printf is not involved as I tried removing that as well. The error happens consistently across three different platforms.

Have you tried to use a [*debugger*](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) to step through the code to see what really happens? — Some programmer dude, Nov 21 '22 at 16:17
you are casting away the constness of a character literal. theis isn UB — pm100, Nov 21 '22 at 16:18
By the way, please don't use the floating-point `pow` function for integer powers. Especially not for powers of two, which can be implemented as simple shift operations. — Some programmer dude, Nov 21 '22 at 16:18
The `while` loop in `ip2array` seems to iterate way too many times whilst it is only supposed to iterate up to 4 times because that is the size of the buffer it writes to each iteration. This is probably causing a stack overflow that is bond to cause crashes. — Havenard, Nov 21 '22 at 16:20
@Someprogrammerdude thank you for the suggestion of bitwise shift for powers, will implement that. — VioletLovesJasmine, Nov 21 '22 at 16:22
It doesn't make sense to do `p++` if there's no trailing dot to absorb after **every** number. Your code would work fine if the syntax were `10.0.0.0.` with a dot at the end. But with no dot, when processing the last number, you do `p++` and skip past the null terminator. Now you're in unknown territory. — Wyck, Nov 21 '22 at 16:23
Thank you, replacing `while (*p != '\0')` with `for (int i = 0; i < 4; p++)` and removing `p++` solved the issue. — VioletLovesJasmine, Nov 21 '22 at 17:03

Vlad from Moscow · Accepted Answer · 2022-11-21T16:29:59.867

2

This while loop

while (*p != '\0') {
    ip_int[i++] = (uint8_t)strtoul(p, &p, 10);
    p++;
}

can invoke undefined behavior due to this statement

    p++;

when *p is equal to '\0' after processing the last number in the string because as a result there will be access memory outside the array ip_str.

edited Nov 21 '22 at 16:29

answered Nov 21 '22 at 16:21

Vlad from Moscow

301,070
26
186
335

The null-terminator is supposed to be readable, that isn't undefined behavior. The fact he is overwriting `ip_int` past it's allocated size is. – Havenard Nov 21 '22 at 16:27
@IngoLeonhardt You are right. I was not attentive. – Vlad from Moscow Nov 21 '22 at 16:28
@Havenard The nul-terminator is not the issue. `p` already points to the nul-terminator after calling `strtoul`. Then `p++` will advance beyond causing the loop to continue. – Gerhardh Nov 21 '22 at 16:28
1

Oh I see. I wasn't aware `strtoul` modified `p`. – Havenard Nov 21 '22 at 16:29
@VioletLovesJasmine strtoul will read memory outside the array ip_str and the function also will try to write values outside the array ip_int that results in undefined behavior. – Vlad from Moscow Nov 21 '22 at 16:53

My code runs fine with test A or test B commented out, but both are run the program crashes

1 Answers1