From b8fdbe53b90f452f7fd46c7f7896dc2dd8a6acba Mon Sep 17 00:00:00 2001 From: Misa Date: Fri, 29 Dec 2023 11:37:01 -0800 Subject: [PATCH] Fix loading plain font.png Unicode indexing out of bounds This code was introduced by Dav999 in abf12632bbc1e310753591289927180e940f9ae2 (PR #1077), but it contains a memory error. I spotted this with Valgrind. The problem comes from the fact that `max_codepoint` is calculated from the width and height of the surface (which will have the same width and height as the source `font.png` from the filesystem). Let's work through an example using a typical 128 by 128 `font.png` and an 8 by 8 glyph. `chars_per_line` is calculated by dividing the width of the image (`temp_surface->w`, or 128) by `f->glyph_w` (8), yielding 16. `max_codepoint` is calculated by first calculating the height of the image divided by the height of the glyph - which here just happens to be the same as `chars_per_line` (16) since we have a square `font.png` - and then multiplying the result by `chars_per_line`. 16 times 16 is 256. Now it is important to recognize here that this is the _amount_ of glyphs in `font.png`. It is _not_ the last codepoint in the image. To see why, consider the fact that codepoint 0 is contained in the image. If we have codepoint 0, then we can't have codepoint 256, because that would imply that we have 257 codepoints, but clearly, we don't. If we try to read codepoint 256, then after working through the calculations to read the glyphs, we would be trying to read from pixel columns 0 through 7 and pixel rows 128 through 135... in a 128 by 128 image... which is clearly incorrect. Therefore, it's incorrect to write the upper bound of the for-loop iterating over every codepoint as `codepoint <= max_codepoint` instead of `codepoint < max_codepoint`. --- desktop_version/src/Font.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop_version/src/Font.cpp b/desktop_version/src/Font.cpp index bd919795..642ff95a 100644 --- a/desktop_version/src/Font.cpp +++ b/desktop_version/src/Font.cpp @@ -407,7 +407,7 @@ static uint8_t load_font(FontContainer* container, const char* name) const uint32_t chars_per_line = temp_surface->w / f->glyph_w; const uint32_t max_codepoint = (temp_surface->h / f->glyph_h) * chars_per_line; - for (uint32_t codepoint = 0x00; codepoint <= max_codepoint; codepoint++) + for (uint32_t codepoint = 0x00; codepoint < max_codepoint; codepoint++) { if (codepoint > 0x7F) {