RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-10-31 16:22:43 +00:00
Code Issues Activity Actions

LibGUI+LibGfx+WindowServer: Sanity check window size dimensions

Browse source 
Previous to this commit, if a `Window` wanted to set its width or height
greater than `INT16_MAX` (32768), both the application owning the Window
and the WindowServer would crash.

The root of this issue is that `size_would_overflow` check in `Bitmap`
has checks for `INT16_MAX`, and `Window.cpp:786` that is called by
`Gfx::Bitmap::create_with_anonymous_buffer` would get null back, then
causing a chain of events resulting in crashes.

Crashes can still occur but with `VERIFY` and `did_misbehave` the
causes of the crash can be more readily identified.
This commit is contained in:
Matthew Jones 2021-06-02 15:06:59 -06:00 • committed by Linus Groh
parent 839aad6e5b
commit ea4116f5bd
3 changed files with 9 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

5
Userland/Libraries/LibGUI/Window.cpp
View file

@ -788,8 +788,11 @@ OwnPtr<WindowBackingStore> Window::create_backing_store(const Gfx::IntSize& size
// FIXME: Plumb scale factor here eventually. // FIXME: Plumb scale factor here eventually.
auto bitmap = Gfx::Bitmap::create_with_anonymous_buffer(format, buffer, size, 1, {}); auto bitmap = Gfx::Bitmap::create_with_anonymous_buffer(format, buffer, size, 1, {});
if (!bitmap) if (!bitmap) {
VERIFY(size.width() <= INT16_MAX);
VERIFY(size.height() <= INT16_MAX);
return {}; return {};
}
return make<WindowBackingStore>(bitmap.release_nonnull()); return make<WindowBackingStore>(bitmap.release_nonnull());
} }

2
Userland/Libraries/LibGfx/Bitmap.cpp
View file

@ -58,7 +58,7 @@ static bool size_would_overflow(BitmapFormat format, const IntSize& size, int sc
if (size.width() < 0 || size.height() < 0) if (size.width() < 0 || size.height() < 0)
return true; return true;
// This check is a bit arbitrary, but should protect us from most shenanigans: // This check is a bit arbitrary, but should protect us from most shenanigans:
if (size.width() >= 32768 || size.height() >= 32768 || scale_factor < 1 || scale_factor > 4) if (size.width() >= INT16_MAX || size.height() >= INT16_MAX || scale_factor < 1 || scale_factor > 4)
return true; return true;
// In contrast, this check is absolutely necessary: // In contrast, this check is absolutely necessary:
size_t pitch = Bitmap::minimum_pitch(size.width() * scale_factor, format); size_t pitch = Bitmap::minimum_pitch(size.width() * scale_factor, format);

4
Userland/Services/WindowServer/ClientConnection.cpp
View file

@ -363,6 +363,10 @@ Messages::WindowServer::SetWindowRectResponse ClientConnection::set_window_rect(
dbgln("ClientConnection: Ignoring SetWindowRect request for fullscreen window"); dbgln("ClientConnection: Ignoring SetWindowRect request for fullscreen window");
return nullptr; return nullptr;
} }
if (rect.width() > INT16_MAX || rect.height() > INT16_MAX) {
did_misbehave(String::formatted("SetWindowRect: Bad window sizing(width={}, height={}), dimension exceeds INT16_MAX", rect.width(), rect.height()).characters());
return nullptr;
}
if (rect.location() != window.rect().location()) { if (rect.location() != window.rect().location()) {
window.set_default_positioned(false); window.set_default_positioned(false);