LibGfx: Load BitmapFont data more safely

Previously, `load_from_memory()` just took a raw pointer to the data,
and then manually calculated offsets from that pointer. Instead, let's
use the MappedFile we already have, to stream in the data, to make
things a bit safer. We also now check that the entire file's data was
read, since if there was data left over, then either the file is bad or
we've done something wrong.

I've moved the code directly into `try_load_from_mapped_file()` since
`load_from_memory()` was only called from there. The extra indirection
wasn't adding anything.
This commit is contained in:
Sam Atkins 2023-10-02 17:48:46 +01:00 committed by Tim Schumacher
parent 253a96277e
commit 80e756daef
2 changed files with 34 additions and 18 deletions

View file

@ -40,6 +40,18 @@ static_assert(AssertSize<FontFileHeader, 80>());
static constexpr size_t s_max_glyph_count = 0x110000;
static constexpr size_t s_max_range_mask_size = s_max_glyph_count / (256 * 8);
}
// FIXME: We define the traits for the const FontFileHeader, because that's the one we use, and defining
// Traits<T> doesn't apply to Traits<T const>. Once that's fixed, remove the const here.
template<>
class AK::Traits<Gfx::FontFileHeader const> : public GenericTraits<Gfx::FontFileHeader const> {
public:
static constexpr bool is_trivially_serializable() { return true; }
};
namespace Gfx {
NonnullRefPtr<Font> BitmapFont::clone() const
{
return MUST(try_clone());
@ -176,9 +188,9 @@ BitmapFont::~BitmapFont()
}
}
ErrorOr<NonnullRefPtr<BitmapFont>> BitmapFont::load_from_memory(u8 const* data)
ErrorOr<NonnullRefPtr<BitmapFont>> BitmapFont::try_load_from_mapped_file(NonnullOwnPtr<Core::MappedFile> mapped_file)
{
auto const& header = *reinterpret_cast<FontFileHeader const*>(data);
auto& header = *TRY(mapped_file->read_in_place<FontFileHeader const>());
if (memcmp(header.magic, "!Fnt", 4))
return Error::from_string_literal("Gfx::BitmapFont::load_from_memory: Incompatible header");
if (header.name[sizeof(header.name) - 1] != '\0')
@ -188,16 +200,29 @@ ErrorOr<NonnullRefPtr<BitmapFont>> BitmapFont::load_from_memory(u8 const* data)
size_t bytes_per_glyph = sizeof(u32) * header.glyph_height;
size_t glyph_count { 0 };
u8* range_mask_start = const_cast<u8*>(data + sizeof(FontFileHeader));
Bytes range_mask { range_mask_start, header.range_mask_size };
// FIXME: These ReadonlyFoo -> Foo casts are awkward, and only needed because BitmapFont is
// sometimes editable and sometimes not. Splitting it into editable/non-editable classes
// would make this a lot cleaner.
ReadonlyBytes readonly_range_mask = TRY(mapped_file->read_in_place<u8 const>(header.range_mask_size));
Bytes range_mask { const_cast<u8*>(readonly_range_mask.data()), readonly_range_mask.size() };
for (size_t i = 0; i < header.range_mask_size; ++i)
glyph_count += 256 * popcount(range_mask[i]);
u8* rows_start = range_mask_start + header.range_mask_size;
Bytes rows { rows_start, glyph_count * bytes_per_glyph };
Span<u8> widths { rows_start + glyph_count * bytes_per_glyph, glyph_count };
ReadonlyBytes readonly_rows = TRY(mapped_file->read_in_place<u8 const>(glyph_count * bytes_per_glyph));
Bytes rows { const_cast<u8*>(readonly_rows.data()), readonly_rows.size() };
ReadonlySpan<u8> readonly_widths = TRY(mapped_file->read_in_place<u8 const>(glyph_count));
Span<u8> widths { const_cast<u8*>(readonly_widths.data()), readonly_widths.size() };
if (!mapped_file->is_eof())
return Error::from_string_literal("Gfx::BitmapFont::load_from_memory: Trailing data in file");
auto name = TRY(String::from_utf8(ReadonlyBytes { header.name, strlen(header.name) }));
auto family = TRY(String::from_utf8(ReadonlyBytes { header.family, strlen(header.family) }));
return adopt_nonnull_ref_or_enomem(new (nothrow) BitmapFont(move(name), move(family), rows, widths, !header.is_variable_width, header.glyph_width, header.glyph_height, header.glyph_spacing, range_mask, header.baseline, header.mean_line, header.presentation_size, header.weight, header.slope));
auto font = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) BitmapFont(move(name), move(family), rows, widths, !header.is_variable_width, header.glyph_width, header.glyph_height, header.glyph_spacing, range_mask, header.baseline, header.mean_line, header.presentation_size, header.weight, header.slope)));
font->m_mapped_file = move(mapped_file);
return font;
}
RefPtr<BitmapFont> BitmapFont::load_from_file(DeprecatedString const& path)
@ -211,13 +236,6 @@ ErrorOr<NonnullRefPtr<BitmapFont>> BitmapFont::try_load_from_file(DeprecatedStri
return try_load_from_mapped_file(move(mapped_file));
}
ErrorOr<NonnullRefPtr<BitmapFont>> BitmapFont::try_load_from_mapped_file(OwnPtr<Core::MappedFile> mapped_file)
{
auto font = TRY(load_from_memory((u8 const*)mapped_file->data()));
font->m_mapped_file = move(mapped_file);
return font;
}
ErrorOr<void> BitmapFont::write_to_file(DeprecatedString const& path)
{
auto stream = TRY(Core::File::open(path, Core::File::OpenMode::Write));

View file

@ -31,7 +31,7 @@ public:
static RefPtr<BitmapFont> load_from_file(DeprecatedString const& path);
static ErrorOr<NonnullRefPtr<BitmapFont>> try_load_from_file(DeprecatedString const& path);
static ErrorOr<NonnullRefPtr<BitmapFont>> try_load_from_mapped_file(OwnPtr<Core::MappedFile>);
static ErrorOr<NonnullRefPtr<BitmapFont>> try_load_from_mapped_file(NonnullOwnPtr<Core::MappedFile>);
ErrorOr<void> write_to_file(DeprecatedString const& path);
ErrorOr<void> write_to_file(NonnullOwnPtr<Core::File> file);
@ -134,8 +134,6 @@ private:
u8 glyph_width, u8 glyph_height, u8 glyph_spacing, Bytes range_mask,
u8 baseline, u8 mean_line, u8 presentation_size, u16 weight, u8 slope, bool owns_arrays = false);
static ErrorOr<NonnullRefPtr<BitmapFont>> load_from_memory(u8 const*);
template<typename T>
int unicode_view_width(T const& view) const;