Skip to content

Commit c00ce2f

Browse files
nicolinusg
authored andcommitted
LibGfx+icc: Verify ICCProfile ID at parse time instead of in icc
Always computing computing the md5 takes some time, but most icc profiles are small. So that's probably fine. If this ends up being a perf problem in the future, or if it ends up rejecting tons of embedded proiles from images, we can row it back. But let's see if we can get away with this first.
1 parent 31af741 commit c00ce2f

File tree

2 files changed

+9
-12
lines changed

2 files changed

+9
-12
lines changed

Userland/Libraries/LibGfx/ICCProfile.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -244,18 +244,20 @@ bool all_bytes_are_zero(const u8 (&bytes)[N])
244244
return true;
245245
}
246246

247-
Optional<Crypto::Hash::MD5::DigestType> parse_profile_id(ICCHeader const& header)
247+
ErrorOr<Optional<Crypto::Hash::MD5::DigestType>> parse_profile_id(ICCHeader const& header, ReadonlyBytes icc_bytes)
248248
{
249249
// ICC v4, 7.2.18 Profile ID field
250250
// "A profile ID field value of zero (00h) shall indicate that a profile ID has not been calculated."
251251
if (all_bytes_are_zero(header.profile_id))
252-
return {};
252+
return Optional<Crypto::Hash::MD5::DigestType> {};
253253

254254
Crypto::Hash::MD5::DigestType id;
255255
static_assert(sizeof(id.data) == sizeof(header.profile_id));
256256
memcpy(id.data, header.profile_id, sizeof(id.data));
257257

258-
// FIXME: Consider comparing read id with compute_id() result and failing if they aren't equal.
258+
auto computed_id = Profile::compute_id(icc_bytes);
259+
if (id != computed_id)
260+
return Error::from_string_literal("ICC::Profile: Invalid profile id");
259261

260262
return id;
261263
}
@@ -399,7 +401,7 @@ ErrorOr<NonnullRefPtr<Profile>> Profile::try_load_from_externally_owned_memory(R
399401
profile->m_flags = Flags { header.profile_flags };
400402
profile->m_rendering_intent = TRY(parse_rendering_intent(header));
401403
profile->m_pcs_illuminant = TRY(parse_pcs_illuminant(header));
402-
profile->m_id = parse_profile_id(header);
404+
profile->m_id = TRY(parse_profile_id(header, bytes));
403405
TRY(parse_reserved(header));
404406

405407
return profile;

Userland/Utilities/icc.cpp

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,9 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
4040
outln("pcs illuminant: {}", profile->pcs_illuminant());
4141

4242
out("id: ");
43-
if (auto id = profile->id(); id.has_value()) {
44-
out("{}", *id);
45-
auto computed = Gfx::ICC::Profile::compute_id(icc_file->bytes());
46-
if (*id == computed)
47-
outln(" (valid)");
48-
else
49-
outln(" (invalid! valid would be {})", computed);
50-
} else
43+
if (auto id = profile->id(); id.has_value())
44+
outln("{}", *id);
45+
else
5146
outln("(not set)");
5247

5348
return 0;

0 commit comments

Comments
 (0)