1

I am having an issue figuring out what wrong with my code below. I run the full code and I do a lot of input testing and errors are being handled as I want them to. I also use stuffs like valgrind, cppchecker to check for bugs which I fixed the bugs. I then decided to use afl-fuzzer to do an advanced bug detection on my codes and then I get a lot of crashes due to the below line of code. However, most of the crashes are due to segmentation fault. but I don't seem to see what is wrong with the code. Any help will be appreciated. Below is the function that keeps giving the error. Which I think has to do with sscanf:

Tree* insert(char* command, Tree* tree) {
    int age;
    char* name = malloc(sizeof(char) * 20);

    if (2 != sscanf(command, "i %d %20s", &age, name)){
        fprintf(stderr, "Failed to parse insert command: not enough parameters filled\n");
       // return NULL;
    }

    if (tree == NULL){
        tree = tree_create();
    }

    tree_insert(tree, age, name);

    return tree;
}

tree_create function

Tree* tree_create(){
Tree *tree = malloc(sizeof(Tree));
tree->root = NULL;

return tree;
}

tree_insert

void tree_insert(Tree* tree, int age, char* name) {
if (tree->root == NULL) {
    Node *node = calloc(1, sizeof(Node));
    node->name = name;
    node->age = age;
    node->isRoot = true;
    node->right = NULL;
    node->left = NULL;
    tree->root = node;

} else {
    node_insert(tree->root, age, name, 1);
}
}
King ja
  • 51
  • 5
  • Which exact line of code does the seg fault happen on? The debugger can tell you that immediately. Also we really need to see a [minimal verifiable example](https://stackoverflow.com/help/minimal-reproducible-example). Otherwise we don't know what `command` values are bring used, we don't know how `tree_create` is implemented, nor how `tree_insert` is implemented etc. – kaylum Jan 08 '20 at 23:34

1 Answers1

4

The primary problem is in the lines:

char* name = malloc(sizeof(char) * 20);

if (2 != sscanf(command, "i %d %20s", &age, name)){

Your conversion specification %20s says that sscanf() can store 20 characters plus a null byte in name, but you only allocated enough space for 19 characters plus a null byte. This 'off-by-one' between the scanf() family of functions and most other functions causes problems, and fuzzers are supposed to find them.

The fix is simple: either change the first 20 to 21 or change the second 20 to 19. Which is better is your judgement call. We don't have enough information to choose which, if either, is the better choice.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278