libzip icon indicating copy to clipboard operation
libzip copied to clipboard

Reading into > 4GB buffer results in data being shifted

Open drussel opened this issue 3 years ago • 1 comments

Describe the Bug zip_int64_t read = zip_fread(f, data, capacity); (on a relatively normal zip file on disk) if capacity > 2^32 then the data ends up in the wrong place in data. Making multiple calls with a reduced capacity works fine.

Expected Behavior The second argument to zip_fread is 64 bit int, so we would expect it to handle large values.

Observed Behavior The filled in data starts at ~data + capacity as opposed to starting at data (it does not start at data, the location where it does end up, I'm not as clear on).

To Reproduce I don't have a stand along example at the moment. Will work on making one.

libzip Version 1.8...1.9.2

Operating System Windows, Mac

drussel avatar Aug 30 '22 17:08 drussel

TEST(LibZip, LargeRead) {
    auto name = V3D::Testing::getOutputPath("largeRead.zip");
    constexpr const zip_int64_t sz = 6000000000;
    constexpr const zip_int64_t maxRead = 100000;
    // constexpr const zip_int64_t maxRead = sz;
    char *d = (char *)malloc(sizeof(char) * sz); // leaks
    for (zip_int64_t i = 0; i < sz; ++i)
        d[i] = i % 1000;
    {
        int errorp = 0;
        boost::filesystem::remove_all(name);
        zip_t *z = zip_open(name.string().c_str(), ZIP_CREATE | ZIP_EXCL, &errorp);
        ASSERT_TRUE(z);
        ASSERT_EQ(errorp, 0);

        auto *s = zip_source_buffer(z, d, sz, 0);
        ASSERT_TRUE(s);
        auto index = zip_file_add(z, "hi", s, ZIP_FL_ENC_UTF_8 | ZIP_FL_OVERWRITE);
        ASSERT_EQ(index, 0);
        zip_close(z);
    }
    {
        int errorp = 0;
        auto z = zip_open(name.string().c_str(), ZIP_RDONLY, &errorp);
        ASSERT_TRUE(z);
        assert(errorp == 0);

        zip_stat_t stat;
        errorp = zip_stat_index(z, 0, 0, &stat);
        ASSERT_NE(errorp, -1);
        auto *f = zip_fopen_index(z, 0, 0);
        ASSERT_TRUE(f);

        char *dr = (char *)malloc(sizeof(char) * sz); // leaks
        zip_int64_t filled = 0;
        while (filled != sz) {
            zip_int64_t read = zip_fread(f, dr + filled, std::min<zip_int64_t>(maxRead, sz - filled));
            ASSERT_NE(read, 0);
            ASSERT_NE(read, -1);
            filled += read;
        }
        for (zip_int64_t i = 0; i < sz; ++i) {
            ASSERT_EQ(d[i], dr[i]) << i;
        }
        zip_fclose(f);
        zip_close(z);
    }
}

Changing to constexpr const zip_int64_t maxRead = sz; will cause it to fail down in the check that we read the right data back.

drussel avatar Aug 30 '22 18:08 drussel

Thanks for the bug report.

dillof avatar Sep 23 '22 11:09 dillof