smasherprog / screen_capture_lite

cross platform screen/window capturing library
MIT License
619 stars 156 forks source link

GetWindows.cpp corrupts stack memory with long window names #30

Closed inantop closed 6 years ago

inantop commented 6 years ago

Tested on commit (Edit: pasted wrong commit earlier) https://github.com/smasherprog/screen_capture_lite/commit/5a82135571a9bea0498d48f9a70c6663bd9498fb

Running "Screen_Capture_Example" using VS2015, a stack corruption occurs in EnumWindowsProc when the user has a window open with a long title (longer than 128 chars, presumably. In my case the window was a chrome page open to https://github.com/smasherprog/Projects_Setup):

    BOOL CALLBACK EnumWindowsProc(_In_ HWND hwnd, _In_ LPARAM lParam)
    {
        char buffer[255];
        GetWindowTextA(hwnd, buffer, sizeof(buffer));
        srch *s = (srch *)lParam;
        std::string name = buffer;
        Window w;
        w.Handle = reinterpret_cast<size_t>(hwnd);
        auto windowrect = SL::Screen_Capture::GetWindowRect(hwnd);
        w.Position.x = windowrect.ClientRect.left;
        w.Position.y = windowrect.ClientRect.top;
        w.Size.x = windowrect.ClientRect.right - windowrect.ClientRect.left;
        w.Size.y = windowrect.ClientRect.bottom - windowrect.ClientRect.top;

        memcpy(w.Name, name.c_str(), name.size() + 1);
        s->Found.push_back(w);
        return TRUE;
    }

buffer is a 255 char array but w.Name is only 128 chars. Re-organizing the code to:

    BOOL CALLBACK EnumWindowsProc(_In_ HWND hwnd, _In_ LPARAM lParam)
    {
        Window w;
        char buffer[sizeof(w.Name)];
        GetWindowTextA(hwnd, buffer, sizeof(buffer));
        ...

fixes the issue and guards against changes to the size of w.Name in the future.

Note also that a compilation issue occurred in GDIFrameProcessor.cpp at ImageRect rect = { 0 }; but presumably this is not a problem for VS2017.

smasherprog commented 6 years ago

Fixed in master. Thanks for pointing that out.