undefined behavior: signed integer overflow in minmea_scan()

[invd]

Fuzzing with libFuzzer shows that the following multiplication can lead to undefined behavior:

https://github.com/kosma/minmea/blob/06ad5a18406c6219290be8c82f966ab585571858/minmea.c#L186

UndefinedBehavior Sanitizer warning:

minmea.c:186:39: runtime error: signed integer overflow: 1000000000 * 10 cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior minmea.c:186:39 in 

Example input: $y$GGA,,.0651205658

[cmorganBE]

@invd would you be interested in opening a PR that adds your fuzzing tests to the test build? That seems like a helpful addition.

[kosma]

@invd, care to share your fuzzing code? I'd love to have that in the repository, so we can test for regressions.

[kosma]

Also I think the solution here is to check for the overflow of both value and scale, since both can overflow independently:

• The scale can overflow with an input like 0.000000000000000000001
• The value can overflow with an input like 1000000000.000000000000

So basically we have to repeat the overflow logic for both value and scale. I will take care of this soon, just need some sleep first - and I also need to set up fuzzing so I can actually verify this fix was successful. Test cases will also be needed.

[invd]

@kosma @cmorganBE I don't have time to do a full PR, but the following should be helpful:

#include <stdio.h>
#include <stdint.h>
#include <string.h>
#include <errno.h>

#include "minmea.h"

// MINMEA_MAX_SENTENCE_LENGTH + 3 + null terminator
#define FUZZER_MAX_BUFFER_LEN MINMEA_MAX_SENTENCE_LENGTH + 3 + 1

int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
    if(size > FUZZER_MAX_BUFFER_LEN - 1) {
        return 0;
    }

    char line[FUZZER_MAX_BUFFER_LEN];

    memcpy(line, data, size);
    line[size] = 0;

    switch (minmea_sentence_id(line, false)) {
        case MINMEA_SENTENCE_RMC: {
            struct minmea_sentence_rmc frame;
            if (minmea_parse_rmc(&frame, line)) {}
        } break;

        case MINMEA_SENTENCE_GGA: {
            struct minmea_sentence_gga frame;
            if (minmea_parse_gga(&frame, line)) {}
        } break;

        case MINMEA_SENTENCE_GSA: {
            struct minmea_sentence_gsa frame;
            if (minmea_parse_gsa(&frame, line)) {}
        } break;

        case MINMEA_SENTENCE_GLL: {
            struct minmea_sentence_gll frame;
            if (minmea_parse_gll(&frame, line)) {}
        } break;

        case MINMEA_SENTENCE_GST: {
            struct minmea_sentence_gst frame;
            if (minmea_parse_gst(&frame, line)) {}
        } break;

        case MINMEA_SENTENCE_GSV: {
            struct minmea_sentence_gsv frame;
            if (minmea_parse_gsv(&frame, line)) {}
        } break;

        case MINMEA_SENTENCE_VTG: {
            struct minmea_sentence_vtg frame;
            if (minmea_parse_vtg(&frame, line)) {}
        } break;

        case MINMEA_SENTENCE_ZDA: {
            struct minmea_sentence_zda frame;
            if (minmea_parse_zda(&frame, line)) {}
        } break;

        default:
            // case 'MINMEA_INVALID', 'MINMEA_UNKNOWN',
        break;
    }

    return 0;
}

The fuzzer harness is based on the example.c.

Basic compilation example without sanitizers with everything in the main folder: clang -fsanitize=fuzzer minmea_fuzzer.c minmea.c -I. -o minmea_fuzzer

I've done some basic adjustments to be compatible with your current master revision a0da280f6451c4ed20b711edb84dc3d258823e20 but not tested it further.

Feel free to include this code under the WTFPL license.

[invd]

I also need to set up fuzzing so I can actually verify this fix was successful.

At least one signed integer overflow is still present:

minmea.c:195:39: runtime error: signed integer overflow: 1000000000 * 10 cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior minmea.c:195:39 in 

[kosma]

Thanks so much! This is enough for me to make a proper PR. I'll get this done ASAP.

[nabhishekn]

Where is the "check.h" file?

[kosma]

It comes from Check Framework:

https://libcheck.github.io/check/

[invd]

@kosma : as of 85439b97dd4984c5efb84ce954b85088e781dae8, the undefined behavior case is still present:

minmea.c:185:39: runtime error: signed integer overflow: 1000000000 * 10 cannot be represented in type 'int'

https://github.com/kosma/minmea/blob/85439b97dd4984c5efb84ce954b85088e781dae8/minmea.c#L185