weggli-rs / weggli

weggli is a fast and robust semantic search tool for C and C++ codebases. It is designed to help security researchers identify interesting functionality in large codebases.
Apache License 2.0
2.32k stars 127 forks source link

Mismatch of generic variable name using "NOT" statement #2

Closed AnisBoss closed 2 years ago

AnisBoss commented 2 years ago

Hello,

While trying to generate a pattern for matching dangling pointer bugs on a random code-base I run into an issue. Explicitly writing the variable name ==> does result in a match. Using a generic variable name ($data) ==> does not result in a match.

ie: Needle: {free($data); not: $data = NULL;} - doesnt match ==> expected behavior : MATCH Needle: {free(data); not: data = NULL;} - does match ==> expected behavior : MATCH

Test file to reproduce the issue (only needle changed):


#[test]
fn not_dangling_correct() {
    let needle = "{free(data); not: data = NULL;}";
    let source = r"
    int32_t random_func_name2(random_struct **handle_ptr)
{
    random_struct_data *data = NULL;
    random_struct *handle = NULL;

    if ((handle_ptr != NULL) && (*handle_ptr != NULL)) {
        handle = *handle_ptr;
        data = (random_struct_data*)handle->_data;

        if (data != NULL) {
            if (data->name != NULL) {
               //TODO
            }
            free(data); //bug is here
        }
        if (handle != NULL) {
            free(handle);
        }
        *handle_ptr = NULL;
    }

    return 0;
}";

    let matches = parse_and_match(needle, source);

    assert_eq!(matches, 1);
}

#[test]
fn not_dangling_wrong() {
    let needle = "{free($data); not: $data = NULL;}";
    let source = r"
    int32_t random_func_name2(random_struct **handle_ptr)
{
    random_struct_data *data = NULL;
    random_struct *handle = NULL;

    if ((handle_ptr != NULL) && (*handle_ptr != NULL)) {
        handle = *handle_ptr;
        data = (random_struct_data*)handle->_data;

        if (data != NULL) {
            if (data->name != NULL) {
               //TODO
            }
            free(data);
        }
        if (handle != NULL) {
            free(handle);
        }
        *handle_ptr = NULL;
    }

    return 0;
}";

    let matches = parse_and_match(needle, source);

    assert_eq!(matches, 1);
}
felixwilhelm commented 2 years ago

Hi Anis,

thanks for using weggli and the detailed bug report. Your description sounds like it could be a real bug, but I have problems reproducing it with your test cases:

not_dangling_wrong() only fails because it finds 2 matches: free(data) and free(handle).
That seems correct?

AnisBoss commented 2 years ago

Hey Felix, Thank you for your quick reply. Actually I did post the wrong test file my bad ; Please find below the correct one :

#[test]
fn not_dangling() {
    let needle = "{free($handle); not: $handle= NULL;}";
    let source = r"
    int32_t random_func_name2(random_struct **handle_ptr)
{
    random_struct_data *data = NULL;
    random_struct *handle = NULL;

    if ((handle_ptr != NULL) && (*handle_ptr != NULL)) {
        handle = *handle_ptr;
        data = (random_struct_data*)handle->_data;

        if (data != NULL) {
            if (data->name != NULL) {
               //TODO
            }
            free(data); //this should not match
            data = NULL ; 
        }
        if (handle != NULL) {
            free(handle); //this should match
        }
        *handle_ptr = NULL;
    }

    return 0;
}";

    let matches = parse_and_match(needle, source);

    assert_eq!(matches, 1);
}

The expected match for this case is 1 instead of 2.

felixwilhelm commented 2 years ago

Hey Anis,

thanks for the new test case. Nice catch :) This should be fixed now.

AnisBoss commented 2 years ago

Hey Felix,

Thank you for the swift fix; I confirm it is now working as intended. \o/ !