Skip to content

Commit 884d8b1

Browse files
dariusarnoldawesomekling
dariusarnold
authored and
awesomekling
committed
LibGfx: Prevent out of bounds access when scaling small Bitmaps
Since the color interpolation requires two pixels in the horizontal and vertical direction to work, 1 pixel wide or high bitmaps would cause a crash when scaling. Fix this by clamping the index into the valid range. Fixes #16047.
1 parent 71d5dc5 commit 884d8b1

File tree

3 files changed

+221
-44
lines changed

3 files changed

+221
-44
lines changed

‎Tests/LibGfx/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ set(TEST_SOURCES
33
BenchmarkJPEGLoader.cpp
44
TestDeltaE.cpp
55
TestFontHandling.cpp
6+
TestGfxBitmap.cpp
67
TestICCProfile.cpp
78
TestImageDecoder.cpp
89
TestRect.cpp

‎Tests/LibGfx/TestGfxBitmap.cpp‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
/*
2+
* Copyright (c) 2022, the SerenityOS developers.
3+
*
4+
* SPDX-License-Identifier: BSD-2-Clause
5+
*/
6+
7+
#include <LibGfx/Bitmap.h>
8+
#include <LibTest/TestCase.h>
9+
10+
TEST_CASE(0001_bitmap_upscaling_width1_height1)
11+
{
12+
auto bitmap = Gfx::Bitmap::create(Gfx::BitmapFormat::BGRx8888, Gfx::IntSize { 1, 1 });
13+
EXPECT_EQ(bitmap.is_error(), false);
14+
bitmap.value()->fill(Gfx::Color::White);
15+
auto scaledBitmap = bitmap.value()->scaled(5.5f, 5.5f);
16+
EXPECT_EQ(scaledBitmap.is_error(), false);
17+
EXPECT_EQ(scaledBitmap.value()->size(), Gfx::IntSize(6, 6));
18+
for (auto x = 0; x < scaledBitmap.value()->width(); x++) {
19+
for (auto y = 0; y < scaledBitmap.value()->height(); y++) {
20+
EXPECT_EQ(scaledBitmap.value()->get_pixel(x, y), bitmap.value()->get_pixel(0, 0));
21+
}
22+
}
23+
}
24+
25+
TEST_CASE(0002_bitmap_upscaling_width1)
26+
{
27+
auto bitmap = Gfx::Bitmap::create(Gfx::BitmapFormat::BGRx8888, Gfx::IntSize { 1, 10 });
28+
EXPECT_EQ(bitmap.is_error(), false);
29+
bitmap.value()->fill(Gfx::Color::White);
30+
auto scaledBitmap = bitmap.value()->scaled(5.5f, 5.5f);
31+
EXPECT_EQ(scaledBitmap.is_error(), false);
32+
EXPECT_EQ(scaledBitmap.value()->size(), Gfx::IntSize(6, 55));
33+
for (auto x = 0; x < scaledBitmap.value()->width(); x++) {
34+
for (auto y = 0; y < scaledBitmap.value()->height(); y++) {
35+
EXPECT_EQ(scaledBitmap.value()->get_pixel(x, y), bitmap.value()->get_pixel(0, 0));
36+
}
37+
}
38+
}
39+
40+
TEST_CASE(0003_bitmap_upscaling_height1)
41+
{
42+
auto bitmap = Gfx::Bitmap::create(Gfx::BitmapFormat::BGRx8888, Gfx::IntSize { 10, 1 });
43+
EXPECT_EQ(bitmap.is_error(), false);
44+
bitmap.value()->fill(Gfx::Color::White);
45+
auto scaledBitmap = bitmap.value()->scaled(5.5f, 5.5f);
46+
EXPECT_EQ(scaledBitmap.is_error(), false);
47+
EXPECT_EQ(scaledBitmap.value()->size(), Gfx::IntSize(55, 6));
48+
for (auto x = 0; x < scaledBitmap.value()->width(); x++) {
49+
for (auto y = 0; y < scaledBitmap.value()->height(); y++) {
50+
EXPECT_EQ(scaledBitmap.value()->get_pixel(x, y), bitmap.value()->get_pixel(0, 0));
51+
}
52+
}
53+
}
54+
55+
TEST_CASE(0004_bitmap_upscaling_keep_width)
56+
{
57+
auto bitmap = Gfx::Bitmap::create(Gfx::BitmapFormat::BGRx8888, Gfx::IntSize { 1, 10 });
58+
EXPECT_EQ(bitmap.is_error(), false);
59+
bitmap.value()->fill(Gfx::Color::White);
60+
auto scaledBitmap = bitmap.value()->scaled(1.f, 5.5f);
61+
EXPECT_EQ(scaledBitmap.is_error(), false);
62+
EXPECT_EQ(scaledBitmap.value()->size(), Gfx::IntSize(1, 55));
63+
for (auto x = 0; x < scaledBitmap.value()->width(); x++) {
64+
for (auto y = 0; y < scaledBitmap.value()->height(); y++) {
65+
EXPECT_EQ(scaledBitmap.value()->get_pixel(x, y), bitmap.value()->get_pixel(0, 0));
66+
}
67+
}
68+
}
69+
70+
TEST_CASE(0005_bitmap_upscaling_keep_height)
71+
{
72+
auto bitmap = Gfx::Bitmap::create(Gfx::BitmapFormat::BGRx8888, Gfx::IntSize { 10, 1 });
73+
EXPECT_EQ(bitmap.is_error(), false);
74+
bitmap.value()->fill(Gfx::Color::White);
75+
auto scaledBitmap = bitmap.value()->scaled(5.5f, 1.f);
76+
EXPECT_EQ(scaledBitmap.is_error(), false);
77+
EXPECT_EQ(scaledBitmap.value()->size(), Gfx::IntSize(55, 1));
78+
for (auto x = 0; x < scaledBitmap.value()->width(); x++) {
79+
for (auto y = 0; y < scaledBitmap.value()->height(); y++) {
80+
EXPECT_EQ(scaledBitmap.value()->get_pixel(x, y), bitmap.value()->get_pixel(0, 0));
81+
}
82+
}
83+
}
84+
85+
TEST_CASE(0006_bitmap_downscaling_width1_height1)
86+
{
87+
auto bitmap = Gfx::Bitmap::create(Gfx::BitmapFormat::BGRx8888, Gfx::IntSize { 10, 10 });
88+
EXPECT_EQ(bitmap.is_error(), false);
89+
bitmap.value()->fill(Gfx::Color::White);
90+
auto scaledBitmap = bitmap.value()->scaled(0.099f, 0.099f);
91+
EXPECT_EQ(scaledBitmap.is_error(), false);
92+
EXPECT_EQ(scaledBitmap.value()->size(), Gfx::IntSize(1, 1));
93+
for (auto x = 0; x < scaledBitmap.value()->width(); x++) {
94+
for (auto y = 0; y < scaledBitmap.value()->height(); y++) {
95+
EXPECT_EQ(scaledBitmap.value()->get_pixel(x, y), bitmap.value()->get_pixel(0, 0));
96+
}
97+
}
98+
}
99+
100+
TEST_CASE(0007_bitmap_downscaling_width1)
101+
{
102+
auto bitmap = Gfx::Bitmap::create(Gfx::BitmapFormat::BGRx8888, Gfx::IntSize { 10, 10 });
103+
EXPECT_EQ(bitmap.is_error(), false);
104+
bitmap.value()->fill(Gfx::Color::White);
105+
auto scaledBitmap = bitmap.value()->scaled(1.f, 0.099f);
106+
EXPECT_EQ(scaledBitmap.is_error(), false);
107+
EXPECT_EQ(scaledBitmap.value()->size(), Gfx::IntSize(10, 1));
108+
for (auto x = 0; x < scaledBitmap.value()->width(); x++) {
109+
for (auto y = 0; y < scaledBitmap.value()->height(); y++) {
110+
EXPECT_EQ(scaledBitmap.value()->get_pixel(x, y), bitmap.value()->get_pixel(0, 0));
111+
}
112+
}
113+
}
114+
115+
TEST_CASE(0008_bitmap_downscaling_height1)
116+
{
117+
auto bitmap = Gfx::Bitmap::create(Gfx::BitmapFormat::BGRx8888, Gfx::IntSize { 10, 10 });
118+
EXPECT_EQ(bitmap.is_error(), false);
119+
bitmap.value()->fill(Gfx::Color::White);
120+
auto scaledBitmap = bitmap.value()->scaled(0.099f, 1.f);
121+
EXPECT_EQ(scaledBitmap.is_error(), false);
122+
EXPECT_EQ(scaledBitmap.value()->size(), Gfx::IntSize(1, 10));
123+
for (auto x = 0; x < scaledBitmap.value()->width(); x++) {
124+
for (auto y = 0; y < scaledBitmap.value()->height(); y++) {
125+
EXPECT_EQ(scaledBitmap.value()->get_pixel(x, y), bitmap.value()->get_pixel(0, 0));
126+
}
127+
}
128+
}

‎Userland/Libraries/LibGfx/Bitmap.cpp‎

Lines changed: 92 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -384,66 +384,114 @@ ErrorOr<NonnullRefPtr<Gfx::Bitmap>> Bitmap::scaled(float sx, float sy) const
384384
auto new_width = new_bitmap->physical_width();
385385
auto new_height = new_bitmap->physical_height();
386386

387-
// The interpolation goes out of bounds on the bottom- and right-most edges.
388-
// We handle those in two specialized loops not only to make them faster, but
389-
// also to avoid four branch checks for every pixel.
387+
if (old_width == 1 && old_height == 1) {
388+
new_bitmap->fill(get_pixel(0, 0));
389+
return new_bitmap;
390+
}
391+
392+
if (old_width > 1 && old_height > 1) {
393+
// The interpolation goes out of bounds on the bottom- and right-most edges.
394+
// We handle those in two specialized loops not only to make them faster, but
395+
// also to avoid four branch checks for every pixel.
396+
for (int y = 0; y < new_height - 1; y++) {
397+
for (int x = 0; x < new_width - 1; x++) {
398+
auto p = static_cast<float>(x) * static_cast<float>(old_width - 1) / static_cast<float>(new_width - 1);
399+
auto q = static_cast<float>(y) * static_cast<float>(old_height - 1) / static_cast<float>(new_height - 1);
400+
401+
int i = floorf(p);
402+
int j = floorf(q);
403+
float u = p - static_cast<float>(i);
404+
float v = q - static_cast<float>(j);
405+
406+
auto a = get_pixel(i, j);
407+
auto b = get_pixel(i + 1, j);
408+
auto c = get_pixel(i, j + 1);
409+
auto d = get_pixel(i + 1, j + 1);
410+
411+
auto e = a.mixed_with(b, u);
412+
auto f = c.mixed_with(d, u);
413+
auto color = e.mixed_with(f, v);
414+
new_bitmap->set_pixel(x, y, color);
415+
}
416+
}
390417

391-
for (int y = 0; y < new_height - 1; y++) {
418+
// Bottom strip (excluding last pixel)
419+
auto old_bottom_y = old_height - 1;
420+
auto new_bottom_y = new_height - 1;
392421
for (int x = 0; x < new_width - 1; x++) {
393422
auto p = static_cast<float>(x) * static_cast<float>(old_width - 1) / static_cast<float>(new_width - 1);
394-
auto q = static_cast<float>(y) * static_cast<float>(old_height - 1) / static_cast<float>(new_height - 1);
395423

396424
int i = floorf(p);
397-
int j = floorf(q);
398425
float u = p - static_cast<float>(i);
399-
float v = q - static_cast<float>(j);
400-
401-
auto a = get_pixel(i, j);
402-
auto b = get_pixel(i + 1, j);
403-
auto c = get_pixel(i, j + 1);
404-
auto d = get_pixel(i + 1, j + 1);
405426

406-
auto e = a.mixed_with(b, u);
407-
auto f = c.mixed_with(d, u);
408-
auto color = e.mixed_with(f, v);
409-
new_bitmap->set_pixel(x, y, color);
427+
auto a = get_pixel(i, old_bottom_y);
428+
auto b = get_pixel(i + 1, old_bottom_y);
429+
auto color = a.mixed_with(b, u);
430+
new_bitmap->set_pixel(x, new_bottom_y, color);
410431
}
411-
}
412-
413-
// Bottom strip (excluding last pixel)
414-
auto old_bottom_y = old_height - 1;
415-
auto new_bottom_y = new_height - 1;
416-
for (int x = 0; x < new_width - 1; x++) {
417-
auto p = static_cast<float>(x) * static_cast<float>(old_width - 1) / static_cast<float>(new_width - 1);
418432

419-
int i = floorf(p);
420-
float u = p - static_cast<float>(i);
433+
// Right strip (excluding last pixel)
434+
auto old_right_x = old_width - 1;
435+
auto new_right_x = new_width - 1;
436+
for (int y = 0; y < new_height - 1; y++) {
437+
auto q = static_cast<float>(y) * static_cast<float>(old_height - 1) / static_cast<float>(new_height - 1);
421438

422-
auto a = get_pixel(i, old_bottom_y);
423-
auto b = get_pixel(i + 1, old_bottom_y);
424-
auto color = a.mixed_with(b, u);
425-
new_bitmap->set_pixel(x, new_bottom_y, color);
426-
}
439+
int j = floorf(q);
440+
float v = q - static_cast<float>(j);
427441

428-
// Right strip (excluding last pixel)
429-
auto old_right_x = old_width - 1;
430-
auto new_right_x = new_width - 1;
431-
for (int y = 0; y < new_height - 1; y++) {
432-
auto q = static_cast<float>(y) * static_cast<float>(old_height - 1) / static_cast<float>(new_height - 1);
442+
auto c = get_pixel(old_right_x, j);
443+
auto d = get_pixel(old_right_x, j + 1);
433444

434-
int j = floorf(q);
435-
float v = q - static_cast<float>(j);
445+
auto color = c.mixed_with(d, v);
446+
new_bitmap->set_pixel(new_right_x, y, color);
447+
}
436448

437-
auto c = get_pixel(old_right_x, j);
438-
auto d = get_pixel(old_right_x, j + 1);
449+
// Bottom-right pixel
450+
new_bitmap->set_pixel(new_width - 1, new_height - 1, get_pixel(physical_width() - 1, physical_height() - 1));
451+
return new_bitmap;
452+
} else if (old_height == 1) {
453+
// Copy horizontal strip multiple times (excluding last pixel to out of bounds).
454+
auto old_bottom_y = old_height - 1;
455+
for (int x = 0; x < new_width - 1; x++) {
456+
auto p = static_cast<float>(x) * static_cast<float>(old_width - 1) / static_cast<float>(new_width - 1);
457+
int i = floorf(p);
458+
float u = p - static_cast<float>(i);
439459

440-
auto color = c.mixed_with(d, v);
441-
new_bitmap->set_pixel(new_right_x, y, color);
442-
}
460+
auto a = get_pixel(i, old_bottom_y);
461+
auto b = get_pixel(i + 1, old_bottom_y);
462+
auto color = a.mixed_with(b, u);
463+
for (int new_bottom_y = 0; new_bottom_y < new_height; new_bottom_y++) {
464+
// Interpolate color only once and then copy into all columns.
465+
new_bitmap->set_pixel(x, new_bottom_y, color);
466+
}
467+
}
468+
for (int new_bottom_y = 0; new_bottom_y < new_height; new_bottom_y++) {
469+
// Copy last pixel of horizontal strip
470+
new_bitmap->set_pixel(new_width - 1, new_bottom_y, get_pixel(physical_width() - 1, old_bottom_y));
471+
}
472+
return new_bitmap;
473+
} else if (old_width == 1) {
474+
// Copy vertical strip multiple times (excluding last pixel to avoid out of bounds).
475+
auto old_right_x = old_width - 1;
476+
for (int y = 0; y < new_height - 1; y++) {
477+
auto q = static_cast<float>(y) * static_cast<float>(old_height - 1) / static_cast<float>(new_height - 1);
478+
int j = floorf(q);
479+
float v = q - static_cast<float>(j);
443480

444-
// Bottom-right pixel
445-
new_bitmap->set_pixel(new_width - 1, new_height - 1, get_pixel(physical_width() - 1, physical_height() - 1));
481+
auto c = get_pixel(old_right_x, j);
482+
auto d = get_pixel(old_right_x, j + 1);
446483

484+
auto color = c.mixed_with(d, v);
485+
for (int new_right_x = 0; new_right_x < new_width; new_right_x++) {
486+
// Interpolate color only once and copy into all rows.
487+
new_bitmap->set_pixel(new_right_x, y, color);
488+
}
489+
}
490+
for (int new_right_x = 0; new_right_x < new_width; new_right_x++) {
491+
// Copy last pixel of vertical strip
492+
new_bitmap->set_pixel(new_right_x, new_height - 1, get_pixel(old_right_x, physical_height() - 1));
493+
}
494+
}
447495
return new_bitmap;
448496
}
449497

0 commit comments

Comments
 (0)