ION28 / BLUESPAWN

An Active Defense and EDR software to empower Blue Teams
GNU General Public License v3.0
1.22k stars 169 forks source link

please add "zip_source_keep(lpZipSource)" to avoid crash #414

Closed z16166 closed 2 years ago

z16166 commented 2 years ago

hi,

I think we should add "zip_source_keep(lpZipSource);" before the following line "auto zip = zip_open_from_source(lpZipSource, 0, &err)" to avoid crash, due to the internal ref counting of "lpZipSource" variable.

If the ref counting is imbalanced, lpZipSource will be freed by "zip_close(zip);", so the subsequent "zip_source_close(lpZipSource);" will crash or cause memory errors.

please refer to the code: https://github.com/ION28/BLUESPAWN/blob/master/BLUESPAWN-win-client/src/scan/YaraScanner.cpp#L29

    zip_error_t err{};
    auto lpZipSource = zip_source_buffer_create(LockResource(hRsrc), SizeofResource(nullptr, hRsrcInfo), 0, &err);
    if(lpZipSource) {
        zip_source_keep(lpZipSource); // we shall add this to balance the ref counting of lpZipSource !!!!
        auto zip = zip_open_from_source(lpZipSource, 0, &err);
        if(zip) {
            auto fdRules = zip_fopen(zip, "data", 0);
            if(fdRules) {
                zip_stat_t stats{};
                if(-1 != zip_stat(zip, "data", ZIP_STAT_SIZE, &stats)) {
                    if(-1 != stats.size) {
                        AllocationWrapper data{ HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
                                                          static_cast<SIZE_T>(stats.size)),
                                                static_cast<SIZE_T>(stats.size), AllocationWrapper::HEAP_ALLOC };
                        if(-1 != zip_fread(fdRules, data, stats.size)) {
                            zip_fclose(fdRules);
                            zip_close(zip);
                            zip_source_close(lpZipSource);

                            return data;
                        }
                    }
                }

                zip_fclose(fdRules);
            }
            zip_close(zip);
        }
        zip_source_close(lpZipSource);
    }
AsparaGus116 commented 2 years ago

All set!