Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

undefined behavior: signed integer overflow in minmea_scan() #56

Open
invd opened this issue Jun 18, 2020 · 8 comments
Open

undefined behavior: signed integer overflow in minmea_scan() #56

invd opened this issue Jun 18, 2020 · 8 comments
Assignees

Comments

@invd
Copy link

invd commented Jun 18, 2020

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

minmea/minmea.c

Line 186 in 06ad5a1

scale *= 10;

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
Copy link
Collaborator

cmorganBE commented May 20, 2022

@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 kosma self-assigned this Jun 2, 2022
@kosma
Copy link
Owner

kosma commented Jun 2, 2022

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

@kosma
Copy link
Owner

kosma commented Jun 2, 2022

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
Copy link
Author

invd commented Jun 3, 2022

@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 a0da280 but not tested it further.

Feel free to include this code under the WTFPL license.

@invd
Copy link
Author

invd commented Jun 3, 2022

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
Copy link
Owner

kosma commented Jun 3, 2022

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

@nabhishekn
Copy link

nabhishekn commented Oct 6, 2022

Where is the "check.h" file?

@kosma
Copy link
Owner

kosma commented Oct 6, 2022

It comes from Check Framework:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants