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.

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

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

radarhere
Copy link
Member

Resolves #8272

#6846 added an error if the bitmap buffer is empty.

Pillow/src/_imagingft.c

Lines 1028 to 1032 in d6cfebd

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

However, this new issue found the buffer is empty for a space character in a particularly font, which seems like a realistic scenario, and has been confirmed as valid by one of the FreeType maintainers - #8272 (comment)

So if the bitmap buffer is empty, this PR just doesn't draw anything, uses the advances to update the position for the next character, and moves on.

@@ -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.

@nulano
Copy link
Contributor

nulano commented Aug 23, 2024

We might want to add a test that triggers the if (stroker != NULL) branch in font_render to make sure that passing such a glyph to the stroker API is safe.

@radarhere
Copy link
Member Author

You're concerned about passing a glyph where bitmap buffer is empty to this block? test_bitmap_font_stroke actually already does this.

@apodtele
Copy link

Note then the glyph outline in question is not empty but degenerate. It contains two points but still produces a blank space. In this particular case, it results in a 0×1 bitmap, aka one empty row. Do not check for bitmap.rows, or check both dimentions

@radarhere
Copy link
Member Author

The glyph outline may not be empty, but the bitmap buffer is. From my understanding, we're safe to skip drawing and just advance if that is the case. I presume the size of the glyph does not affect the advance.

@apodtele
Copy link

Correct, just advance if the buffer is NULL regardless of its dimensions. You have sufficient error checking from FreeType calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bitmap missing for glyph
3 participants