Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

If bitmap buffer is empty, do not render anything #8324

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+137 −138
Open

If bitmap buffer is empty, do not render anything #8324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f9f7ba4
Do not raise error if bitmap buffer is empty
radarhere Aug 23, 2024
94c3ee6
Merge branch 'main' into bitmap_buffer
radarhere Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions Tests/test_font_crash.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
from __future__ import annotations

import pytest

from PIL import Image, ImageDraw, ImageFont

from .helper import skip_unless_feature
from .helper import skip_unless_feature_version


class TestFontCrash:
Expand All @@ -17,8 +15,7 @@ def _fuzz_font(self, font: ImageFont.FreeTypeFont) -> None:
draw.multiline_textbbox((10, 10), "ABC\nAaaa", font, stroke_width=2)
draw.text((10, 10), "Test Text", font=font, fill="#000")

@skip_unless_feature("freetype2")
@skip_unless_feature_version("freetype2", "2.12.0")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change the version here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test intended to catch OSError: Bitmap missing for glyph from

Pillow/src/_imagingft.c

Lines 1028 to 1032 in b6f90c4

// Null buffer, is dereferenced in FT_Bitmap_Convert
if (!bitmap.buffer && bitmap.rows) {
PyErr_SetString(PyExc_OSError, "Bitmap missing for glyph");
goto glyph_error;
}

For FreeType2 versions before 2.12.0, FT_New_Face is returning an Unknown_File_Format error. It does that even on main at the moment, it's just not obvious because both are OSError. If I increase the specificity of the test, you can see this.

FreeType2 2.12.0 was released in March 2022, before #6846, so I expect this has been the case the whole time. Now that I've fixed the OSError: Bitmap missing for glyph error, the other error becomes obvious.

This was a font generated by OSS-Fuzz, so its purpose is not to be valid, but merely to trigger a security problem. We could modify the file to be valid, but in principle it would seem to be better to not modify test scenarios over time.

I suppose I could modify the test to run on FreeType2 < 2.12.0 as well, and just catch the error for that scenario if you want.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the explanation.

def test_segfault(self) -> None:
with pytest.raises(OSError):
font = ImageFont.truetype("Tests/fonts/fuzz_font-5203009437302784")
self._fuzz_font(font)
font = ImageFont.truetype("Tests/fonts/fuzz_font-5203009437302784")
self._fuzz_font(font)
264 changes: 133 additions & 131 deletions src/_imagingft.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1031,130 +1031,99 @@
yy = -(py + glyph_slot->bitmap_top);
}

// Null buffer, is dereferenced in FT_Bitmap_Convert
if (!bitmap.buffer && bitmap.rows) {
PyErr_SetString(PyExc_OSError, "Bitmap missing for glyph");
goto glyph_error;
}

/* convert non-8bpp bitmaps */
switch (bitmap.pixel_mode) {
case FT_PIXEL_MODE_MONO:
convert_scale = 255;
break;
case FT_PIXEL_MODE_GRAY2:
convert_scale = 255 / 3;
break;
case FT_PIXEL_MODE_GRAY4:
convert_scale = 255 / 15;
break;
default:
convert_scale = 1;
}
switch (bitmap.pixel_mode) {
case FT_PIXEL_MODE_MONO:
case FT_PIXEL_MODE_GRAY2:
case FT_PIXEL_MODE_GRAY4:
if (!bitmap_converted_ready) {
FT_Bitmap_Init(&bitmap_converted);
bitmap_converted_ready = 1;
}
error = FT_Bitmap_Convert(library, &bitmap, &bitmap_converted, 1);
if (error) {
geterror(error);
goto glyph_error;
}
bitmap = bitmap_converted;
/* bitmap is now FT_PIXEL_MODE_GRAY, fall through */
case FT_PIXEL_MODE_GRAY:
break;
case FT_PIXEL_MODE_BGRA:
if (color) {
if (bitmap.buffer) {
/* convert non-8bpp bitmaps */
switch (bitmap.pixel_mode) {
case FT_PIXEL_MODE_MONO:
convert_scale = 255;
break;
}
/* we didn't ask for color, fall through to default */
default:
PyErr_SetString(PyExc_OSError, "unsupported bitmap pixel mode");
goto glyph_error;
}
case FT_PIXEL_MODE_GRAY2:
convert_scale = 255 / 3;
break;
case FT_PIXEL_MODE_GRAY4:
convert_scale = 255 / 15;
break;
default:
convert_scale = 1;
}
switch (bitmap.pixel_mode) {
case FT_PIXEL_MODE_MONO:
case FT_PIXEL_MODE_GRAY2:
case FT_PIXEL_MODE_GRAY4:
if (!bitmap_converted_ready) {
FT_Bitmap_Init(&bitmap_converted);
bitmap_converted_ready = 1;
}
error = FT_Bitmap_Convert(library, &bitmap, &bitmap_converted, 1);
if (error) {
geterror(error);
goto glyph_error;

Check warning on line 1060 in src/_imagingft.c

View check run for this annotation

Codecov / codecov/patch

src/_imagingft.c#L1059-L1060 

Added lines #L1059 - L1060 were not covered by tests
}
bitmap = bitmap_converted;
/* bitmap is now FT_PIXEL_MODE_GRAY, fall through */
case FT_PIXEL_MODE_GRAY:
break;
case FT_PIXEL_MODE_BGRA:
if (color) {
break;
}
/* we didn't ask for color, fall through to default */
default:
PyErr_SetString(PyExc_OSError, "unsupported bitmap pixel mode");
goto glyph_error;

Check warning on line 1073 in src/_imagingft.c

View check run for this annotation

Codecov / codecov/patch

src/_imagingft.c#L1072-L1073 

Added lines #L1072 - L1073 were not covered by tests
}

/* clip glyph bitmap width to target image bounds */
x0 = 0;
x1 = bitmap.width;
if (xx < 0) {
x0 = -xx;
}
if (xx + x1 > im->xsize) {
x1 = im->xsize - xx;
}
/* clip glyph bitmap width to target image bounds */
x0 = 0;
x1 = bitmap.width;
if (xx < 0) {
x0 = -xx;

Check warning on line 1080 in src/_imagingft.c

View check run for this annotation

Codecov / codecov/patch

src/_imagingft.c#L1080 

Added line #L1080 was not covered by tests
}
if (xx + x1 > im->xsize) {
x1 = im->xsize - xx;

Check warning on line 1083 in src/_imagingft.c

View check run for this annotation

Codecov / codecov/patch

src/_imagingft.c#L1083 

Added line #L1083 was not covered by tests
}

source = (unsigned char *)bitmap.buffer;
for (bitmap_y = 0; bitmap_y < bitmap.rows; bitmap_y++, yy++) {
/* clip glyph bitmap height to target image bounds */
if (yy >= 0 && yy < im->ysize) {
/* blend this glyph into the buffer */
int k;
unsigned char *target;
unsigned int tmp;
if (color) {
/* target[RGB] returns the color, target[A] returns the mask */
/* target bands get split again in ImageDraw.text */
target = (unsigned char *)im->image[yy] + xx * 4;
} else {
target = im->image8[yy] + xx;
}
if (color && bitmap.pixel_mode == FT_PIXEL_MODE_BGRA) {
/* paste color glyph */
for (k = x0; k < x1; k++) {
unsigned int src_alpha = source[k * 4 + 3];

/* paste only if source has data */
if (src_alpha > 0) {
/* unpremultiply BGRa */
int src_red =
CLIP8((255 * (int)source[k * 4 + 2]) / src_alpha);
int src_green =
CLIP8((255 * (int)source[k * 4 + 1]) / src_alpha);
int src_blue =
CLIP8((255 * (int)source[k * 4 + 0]) / src_alpha);

/* blend required if target has data */
if (target[k * 4 + 3] > 0) {
/* blend RGBA colors */
target[k * 4 + 0] =
BLEND(src_alpha, target[k * 4 + 0], src_red, tmp);
target[k * 4 + 1] =
BLEND(src_alpha, target[k * 4 + 1], src_green, tmp);
target[k * 4 + 2] =
BLEND(src_alpha, target[k * 4 + 2], src_blue, tmp);
target[k * 4 + 3] = CLIP8(
src_alpha +
MULDIV255(target[k * 4 + 3], (255 - src_alpha), tmp)
);
} else {
/* paste unpremultiplied RGBA values */
target[k * 4 + 0] = src_red;
target[k * 4 + 1] = src_green;
target[k * 4 + 2] = src_blue;
target[k * 4 + 3] = src_alpha;
}
}
}
} else if (bitmap.pixel_mode == FT_PIXEL_MODE_GRAY) {
source = (unsigned char *)bitmap.buffer;
for (bitmap_y = 0; bitmap_y < bitmap.rows; bitmap_y++, yy++) {
/* clip glyph bitmap height to target image bounds */
if (yy >= 0 && yy < im->ysize) {
/* blend this glyph into the buffer */
int k;
unsigned char *target;
unsigned int tmp;
if (color) {
unsigned char *ink = (unsigned char *)&foreground_ink;
/* target[RGB] returns the color, target[A] returns the mask */
/* target bands get split again in ImageDraw.text */
target = (unsigned char *)im->image[yy] + xx * 4;
} else {
target = im->image8[yy] + xx;
}
if (color && bitmap.pixel_mode == FT_PIXEL_MODE_BGRA) {
/* paste color glyph */
for (k = x0; k < x1; k++) {
unsigned int src_alpha = source[k] * convert_scale;
unsigned int src_alpha = source[k * 4 + 3];

/* paste only if source has data */
if (src_alpha > 0) {
/* unpremultiply BGRa */
int src_red =
CLIP8((255 * (int)source[k * 4 + 2]) / src_alpha);
int src_green =
CLIP8((255 * (int)source[k * 4 + 1]) / src_alpha);
int src_blue =
CLIP8((255 * (int)source[k * 4 + 0]) / src_alpha);

/* blend required if target has data */
if (target[k * 4 + 3] > 0) {
/* blend RGBA colors */
target[k * 4 + 0] = BLEND(
src_alpha, target[k * 4 + 0], ink[0], tmp
src_alpha, target[k * 4 + 0], src_red, tmp
);
target[k * 4 + 1] = BLEND(
src_alpha, target[k * 4 + 1], ink[1], tmp
src_alpha, target[k * 4 + 1], src_green, tmp
);
target[k * 4 + 2] = BLEND(
src_alpha, target[k * 4 + 2], ink[2], tmp
src_alpha, target[k * 4 + 2], src_blue, tmp
);
target[k * 4 + 3] = CLIP8(
src_alpha +
Expand All @@ -1163,35 +1132,68 @@
)
);
} else {
target[k * 4 + 0] = ink[0];
target[k * 4 + 1] = ink[1];
target[k * 4 + 2] = ink[2];
/* paste unpremultiplied RGBA values */
target[k * 4 + 0] = src_red;
target[k * 4 + 1] = src_green;
target[k * 4 + 2] = src_blue;
target[k * 4 + 3] = src_alpha;
}
}
}
} else {
for (k = x0; k < x1; k++) {
unsigned int src_alpha = source[k] * convert_scale;
if (src_alpha > 0) {
target[k] =
target[k] > 0
? CLIP8(
src_alpha +
MULDIV255(
target[k], (255 - src_alpha), tmp
} else if (bitmap.pixel_mode == FT_PIXEL_MODE_GRAY) {
if (color) {
unsigned char *ink = (unsigned char *)&foreground_ink;
for (k = x0; k < x1; k++) {
unsigned int src_alpha = source[k] * convert_scale;
if (src_alpha > 0) {
if (target[k * 4 + 3] > 0) {
target[k * 4 + 0] = BLEND(
src_alpha, target[k * 4 + 0], ink[0], tmp
);
target[k * 4 + 1] = BLEND(
src_alpha, target[k * 4 + 1], ink[1], tmp
);
target[k * 4 + 2] = BLEND(
src_alpha, target[k * 4 + 2], ink[2], tmp
);
target[k * 4 + 3] = CLIP8(
src_alpha + MULDIV255(
target[k * 4 + 3],
(255 - src_alpha),
tmp
)
);
} else {
target[k * 4 + 0] = ink[0];
target[k * 4 + 1] = ink[1];
target[k * 4 + 2] = ink[2];
target[k * 4 + 3] = src_alpha;
}
}
}
} else {
for (k = x0; k < x1; k++) {
unsigned int src_alpha = source[k] * convert_scale;
if (src_alpha > 0) {
target[k] =
target[k] > 0
? CLIP8(
src_alpha +
MULDIV255(
target[k], (255 - src_alpha), tmp
)
)
)
: src_alpha;
: src_alpha;
}
}
}
} else {
PyErr_SetString(PyExc_OSError, "unsupported bitmap pixel mode");
goto glyph_error;

Check warning on line 1192 in src/_imagingft.c

View check run for this annotation

Codecov / codecov/patch

src/_imagingft.c#L1191-L1192 

Added lines #L1191 - L1192 were not covered by tests
}
} else {
PyErr_SetString(PyExc_OSError, "unsupported bitmap pixel mode");
goto glyph_error;
}
source += bitmap.pitch;
}
source += bitmap.pitch;
}
x += glyph_info[i].x_advance;
y += glyph_info[i].y_advance;
Expand Down
Loading