intel / QATzip

Compression Library accelerated by Intel® QuickAssist Technology
https://developer.intel.com/quickassist
Other
140 stars 51 forks source link

double free on fi object when opendir fails in function itemListCreate() #93

Closed ColinIanKing closed 8 months ago

ColinIanKing commented 1 year ago

When opendir fails in function itemListCreate(), we have a double free on the fi object. It's a bit subtle, I will try an explain how:

        if (fi->isDir || fi->isEmpty) {
            qzListAdd(dirs_head, (void **)&fi);  // add it to directory list

            while (dir_index < dirs_head->total) {
                Qz7zFileItem_T  *processing =
                    (Qz7zFileItem_T *)qzListGet(dirs_head, dir_index);

                if (processing == NULL) {
                    QZ_DEBUG("qzListGet got NULL!\n");
                    goto error;
                }
                QZ_DEBUG("processing: %s\n", processing->fileName);

                if (processing->isDir) {
                    struct dirent *dentry;
                    dirp = opendir(processing->fileName);
                    if (!dirp) {
                        QZ_ERROR("errors ocurs: %s \n", strerror(errno));
                        goto error;
                    }

The fi object is added to the dirs_head list in the call qzListAdd(dirs_head, (void **)&fi); a little later on we have opendir() being called and the error is handled by jumping to label error. This does the following:

error:
    if (fi) {
        free(fi);
    }
    if (res) {
        itemListDestroy(res);
    }
    return NULL;

The allocated fi object is free'd. Since res is allocated we destroy it with the call to itemListDestroy which calls qzListDestory(), however, this free's fi again because fi has been previously added to the dirs_head list.

void itemListDestroy(Qz7zItemList_T *p)
{
    QzListHead_T  *h;
    for (int i = 0; i < 2; ++i) {
        h = p->items[i];
        qzListDestroy(h);
    }
...

in qzListDestroy we have:


void qzListDestroy(QzListHead_T *head)
{
    Qz7zFileItem_T  *fi;
    QzListNode_T  *node, *tmp_node;
    node = head->next;
    do {
        for (int j = 0; j < node->used; ++j) {
            fi = node->items[j];
            free(fi->fileName);
            free(fi);                            // <--- DOUBLE FREE 
        }
        free(node->items);
        tmp_node = node;
        node = node->next;
        free(tmp_node);
    } while (node);
    free(head);
}
GarenJian-Intel commented 9 months ago

Will fix it in the next release.

GarenJian-Intel commented 8 months ago

Completed!