several potential bugs of NULL Pointer Dereference

[ash1852]

hello .I found some bugs of NULL Pointer Dereference in source code of check-0.10.0.I know it may be a old version,but I find out that these potential bugs are still exist in current version.I think in this case,os information and lib information is not important because bugs are obtained by static analysis.

would you help me check these bugs are true?thank you very much.

there are several potential bugs of NULL Pointer Dereference :

==============================================================================

step 1 : In file check/src/check_list.c , function check_list_val line 121 : Return null to caller

step 2 : In file check/src/check_run.c , function srunner_iterate_tcase_tfuns line 247 : Function check_list_val executes and stores the return value to tfun (tfun can be null)

step 3 : In file check/src/check_run.c , function srunner_iterate_tcase_tfuns line 249: Load value from tfun->loop_start and assign to i

==============================================================================

step 1 : In file check/src/check_list.c , function check_list_val line 121: Return null to caller

step 2 : In file check/src/check_run.c , function srunner_run_teardown line 366: Function check_list_val executes and stores the return value to fixture (fixture can be null)

step 3 : In file check/src/check_run.c , function srunner_run_teardown line 373/383 : Load value from fixture->fun

==============================================================================

step 1 : In file check/src/check_list.c , function check_list_val line 122 : Return null to caller

step 2 : In file check/src/check_run.c , function srunner_run_teardown line 337 : Function check_list_val executes and stores the return value to fixture (fixture can be null)

step 3 : In file check/src/check_run.c , function srunner_run_teardown line 354 : Load value from fixture->fun

==============================================================================

step 1 : In file check/src/check_list.c , function check_list_val line 121: Return null to caller

step 2 : In file check/src/check_log.c , function srunner_send_evt line 171 : Function check_list_val executes and stores the return value to lg (lg can be null)

step 3 : In file check/src/check_log.c , function srunner_send_evt line 172 : Load value from lg->lfile

==============================================================================

step 1 : In file check/src/check_list.c , function check_list_val line 121 : Return null to caller

step 2 : In file check/src/check_run.c , function srunner_iterate_suites line 188 : Function check_list_val executes and stores the return value to s (s can be null)

step 3 : In file check/src/check_run.c , function srunner_iterate_suites line 190 : Load value from s->name

==============================================================================

step 1 : In file check/src/check_list.c , function check_list_val line 121 : Return null to caller

step 2 : In file check/src/check.c , function suite_tcase line 88 : Function check_list_val executes and stores the return value to tc (tc can be null)

step 3 : In file check/src/check.c , function suite_tcase line 89 : Load value from tc->name

==============================================================================

step 1 : In file check/src/check_list.c , function check_list_val line 121 : Return null to caller

step 2 : In file check/src/check.c , function srunner_failures line 487 : Function check_list_val executes and stores the return value to tr (tr can be null)

step 3 : In file check/src/check.c , function srunner_failures line 489 : Load value from tr->rtype

==============================================================================

step 1 : In file check/src/check_list.c , function check_list_val line 121 : Return null to caller

step 2 : In file check/src/check.c , function srunner_free line 457: Function check_list_val executes and stores the return value to tr (tr can be null)

step 3 : In file check/src/check.c , function srunner_free line 458: tr is used as the 1st parameter in function tr_free (tr can be null)

step 4 : In file check/src/check.c , function tr_free line 541/542/543 : Load value from tr->member

==============================================================================

step 1 : In file check/src/check_list.c , function check_list_val line 122 : Return null to caller

step 2 : In file check/src/check_log.c , function srunner_end_logging line 543 : Function check_list_val executes and stores the return value to lg (lg can be null)

step 3 : In file check/src/check_log.c , function srunner_end_logging line 545 : Load value from lg->close

==============================================================================

step 1 : In file check/src/check_list.c , function check_list_val line 121 : Return null to caller

step 2 : In file check/src/check.c , function suite_tcase line 88 : Function check_list_val executes and stores the return value to tc (tc can be null)

step 3 : In file check/src/check.c , function suite_tcase line 89 : Load value from tc->name

[mikkoi]

Which static analysis software was used?

[ash1852]

Which static analysis software was used?

The static analyzer was developed by us

[mikkoi]

I checked some of the cases. They are not bugs, as far as I can see. Let's look at the last one:

The function definition:

void *check_list_val(List * lp)
{
    if(lp == NULL)
        return NULL;
    if(lp->current == -1 || lp->current > lp->last)
        return NULL;

    return lp->data[lp->current];
}

The function is used check/src/check.c:

int suite_tcase(Suite * s, const char *tcname)
{
    List *l;

    if(s == NULL)
        return 0;

    l = s->tclst;
    for(check_list_front(l); !check_list_at_end(l); check_list_advance(l))
    {
        TCase *tc = (TCase *)check_list_val(l);
        if(strcmp(tcname, tc->name) == 0)
            return 1;
    }

    return 0;
}

We can see the declaration List *l and later l is assigned the linked list. It is then being used inside the for loop. But the loop itself controls that we only access valid pointers. This is done with check_list_at_end(l). If this function returns positive, then the list is advanced by placing the new pointer to l->current.

I would say that here the static analyser produced a fault positive but it is on the right track. These could be bugs. A linked list is a powerful and fast way to keep a list in C but it is easy to make mistakes when using a linked list. In this particular case, this list maintains its own current pointer. This is often not the case. It is more often the case that the pointer to the current node is maintained by the loop in the calling function. This might be another reason why the static analyser is confused. As a side note, this list structure is absolutely not useful for threaded code (parallel processing).