From 7ae97c9fc40cecc527296c760d1a8a26d0734343 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 8 Jan 2023 07:07:21 -0500 Subject: [PATCH] LibGfx+icc: Look at profile_size field This trims the input bytes to the profile size stored in the file. Alternatively, we could reject files where the stored size doesn't match the handed in size. But ICC profiles can be embedded in other files, and those could conceivably pad the ICC profile data some. --- Userland/Libraries/LibGfx/ICCProfile.cpp | 21 +++++++++++++++++++++ Userland/Libraries/LibGfx/ICCProfile.h | 2 ++ Userland/Utilities/icc.cpp | 6 ++++++ 3 files changed, 29 insertions(+) diff --git a/Userland/Libraries/LibGfx/ICCProfile.cpp b/Userland/Libraries/LibGfx/ICCProfile.cpp index 3f60eef6c4..964a83d362 100644 --- a/Userland/Libraries/LibGfx/ICCProfile.cpp +++ b/Userland/Libraries/LibGfx/ICCProfile.cpp @@ -116,6 +116,22 @@ struct ICCHeader { }; static_assert(sizeof(ICCHeader) == 128); +ErrorOr parse_size(ICCHeader const& header, ReadonlyBytes icc_bytes) +{ + // ICC v4, 7.2.2 Profile size field + // "The value in the profile size field shall be the exact size obtained by combining the profile header, + // the tag table, and the tagged element data, including the pad bytes for the last tag." + + // Valid files have enough data for profile header and tag table entry count. + if (header.profile_size < sizeof(ICCHeader) + sizeof(u32)) + return Error::from_string_literal("ICC::Profile: Profile size too small"); + + if (header.profile_size > icc_bytes.size()) + return Error::from_string_literal("ICC::Profile: Profile size larger than input data"); + + return header.profile_size; +} + Optional parse_preferred_cmm_type(ICCHeader const& header) { // ICC v4, 7.2.3 Preferred CMM type field @@ -504,6 +520,7 @@ ErrorOr> Profile::try_load_from_externally_owned_memory(R auto header = *bit_cast(bytes.data()); TRY(parse_file_signature(header)); + profile->m_on_disk_size = TRY(parse_size(header, bytes)); profile->m_preferred_cmm_type = parse_preferred_cmm_type(header); profile->m_version = TRY(parse_version(header)); profile->m_device_class = TRY(parse_device_class(header)); @@ -521,6 +538,10 @@ ErrorOr> Profile::try_load_from_externally_owned_memory(R profile->m_id = TRY(parse_profile_id(header, bytes)); TRY(parse_reserved(header)); + bytes = bytes.trim(header.profile_size); + bytes = bytes.slice(sizeof(ICCHeader)); + // FIXME: Read tag table. + return profile; } diff --git a/Userland/Libraries/LibGfx/ICCProfile.h b/Userland/Libraries/LibGfx/ICCProfile.h index 8bb97204ad..e015412af4 100644 --- a/Userland/Libraries/LibGfx/ICCProfile.h +++ b/Userland/Libraries/LibGfx/ICCProfile.h @@ -223,6 +223,7 @@ public: // For non-DeviceLink profiles, always PCSXYZ or PCSLAB. ColorSpace connection_space() const { return m_connection_space; } + u32 on_disk_size() const { return m_on_disk_size; } time_t creation_timestamp() const { return m_creation_timestamp; } PrimaryPlatform primary_platform() const { return m_primary_platform; } Flags flags() const { return m_flags; } @@ -237,6 +238,7 @@ public: static Crypto::Hash::MD5::DigestType compute_id(ReadonlyBytes); private: + u32 m_on_disk_size { 0 }; Optional m_preferred_cmm_type; Version m_version; DeviceClass m_device_class; diff --git a/Userland/Utilities/icc.cpp b/Userland/Utilities/icc.cpp index ac786a87fe..5e22e04e60 100644 --- a/Userland/Utilities/icc.cpp +++ b/Userland/Utilities/icc.cpp @@ -67,5 +67,11 @@ ErrorOr serenity_main(Main::Arguments arguments) out_optional("creator", profile->creator()); out_optional("id", profile->id()); + size_t profile_disk_size = icc_file->size(); + if (profile_disk_size != profile->on_disk_size()) { + VERIFY(profile_disk_size > profile->on_disk_size()); + outln("{} trailing bytes after profile data", profile_disk_size - profile->on_disk_size()); + } + return 0; }