lvandeve / lodepng

PNG encoder and decoder in C and C++.
zlib License
2.11k stars 426 forks source link

lodepng::State cause Stack Corruption #191

Open OshidaBCF opened 3 months ago

OshidaBCF commented 3 months ago

In a school project our teacher gave us a "very old" version of lodepng (20141130)

Trying to use lodepng::State caused a stack corruption "around" the variable.

so i updated the lib hoping that it would fix it and it sadly didn't

BUT, the new version has decode and encode functions that doesn't need a state variable

using those two functions cause no issue and now works perfectly.

But i am still very confused about this stack corruption, i don't know what i can give to help so ask me and i'll try to provide

lvandeve commented 3 months ago

Hey, in which environment is this? Which C++ compiler is used and how do you see the stack corruption (e.g. which error message by which tool)?

Thanks!

OshidaBCF commented 3 months ago

i don't know what different compiler, but it should be the default one (gcc ?) that comes installed with visual studio 2022 v143

C++ language Standard is C++20

the error appear at the end of the function where the "lodepng::State state" variable is created

the error only appear when the function exit (maybe destructor of State ?)

this is the error i have image

the actual variable name sometime ossilated between state and pngwidth depending in what order i created them, if i create them with state last the message would only say "state" and never the others

This here is the function in question, it's very different now with the new stateless functions but here it was when it had the issue

bool GCImage::LoadPNG(const std::string& filename)
{
    GCFile imageFile = GCFile(filename.c_str(), "rb");
    std::vector<uint8_t> buffer;
    if (!imageFile.Read(buffer, imageFile.size))
    {
        std::printf("Error opening file\n");
        Close();
        return 1;
    }

    UI32 pngWidth = 0;
    UI32 pngHeight = 0;
    lodepng::State state;
    if (lodepng_inspect(&pngWidth, &pngHeight, &state, buffer.data(), imageFile.size))
    {
        return false;
    }
    switch (state.info_png.color.colortype)
    {
    case LCT_RGBA:
    case LCT_GREY_ALPHA:
    case LCT_PALETTE:
        m_bitCount = 32;
        break;
    case LCT_RGB:
    case LCT_GREY:
        m_bitCount = 24;
        break;
    default:
        return false;
        break;
    }

    lodepng_state_cleanup(&state);
    UI32 error = lodepng_decode32(&m_rgba, (UI32*)&m_width, (UI32*)&m_height, buffer.data(), imageFile.size);
    if (error)
    {
        Close();
        return false;
    }

    m_rowStride = m_width * 4;
    m_size = m_rowStride * m_height;
    m_bits = m_width * m_height;
    m_channels = m_bitCount / 8;
    for (size_t i = 0; i < m_size; i++)
    {
        data.emplace_back(m_rgba[i]);
    }
    return true;
}

Note that even if there was JUST the lodepng::State state line in the function it would give the error

visual apparently make "backups" of critical data so the code could actually continue running when you pass the error.

lvandeve commented 3 months ago

The issue might be that the call to lodepng_state_cleanup is not needed: the destructor already does that and doing it twice could cause the issue (free'ing memory twice)

OshidaBCF commented 3 months ago

perhaps but the stack error would happen even if that line was commented out

i tested the function just like this and it still had the same error

bool GCImage::LoadPNG(const std::string& filename)
{
    GCFile imageFile = GCFile(filename.c_str(), "rb");
    std::vector<uint8_t> buffer;
    if (!imageFile.Read(buffer, imageFile.size))
    {
        std::printf("Error opening file\n");
        Close();
        return 1;
    }

    UI32 pngWidth = 0;
    UI32 pngHeight = 0;
    lodepng::State state;
}
lvandeve commented 3 months ago

Does any further interesting information show when expanding the 'Show Call Stack' or 'Copy Details' options in the Exception Thrown dialog?

Is there any other code executed other than the function shown? Is there a minimal test case where the issue occurs, e.g. when having only a single main function with only a call to lodepng::State state; in it, does the issue still occur?

I couldn't reproduce the issue yet (I don't have the same environment though, but e.g. use gcc and clang with memory debugging tools like valgrind and the fsanitize options of clang), so any extra information is useful. Thanks!