mm2 / Little-CMS

A free, open source, CMM engine. It provides fast transforms between ICC profiles.
https://www.littlecms.com
MIT License
549 stars 174 forks source link

MD5 computation of Profile ID seems to zero wrong header field #447

Closed Overruler closed 4 months ago

Overruler commented 4 months ago

cmsmd5.c temporarily sets zero the following header fields:

    memset(&Icc ->attributes, 0, sizeof(Icc ->attributes));
    Icc ->RenderingIntent = 0;
    memset(&Icc ->ProfileID, 0, sizeof(Icc ->ProfileID));

A comment refers to an ICC specification:

// Assuming io points to an ICC profile, compute and store MD5 checksum
// In the header, rendering intentent, attributes and ID should be set to zero
// before computing MD5 checksum (per 6.1.13 in ICC spec)

6.1.13 looks like a reference to an ICC v2 profile specification, but I'm unable to find such a specification. The latest v2 specification doesn't specify the byte range that should store the Profile ID field. The v4 specification specifies the Profile ID computation differently:

7.2.18 Profile ID field (bytes 84 to 99) This field, if not zero (00h), shall hold the Profile ID. The Profile ID shall be calculated using the MD5 fingerprinting method as defined in Internet RFC 1321. The entire profile, whose length is given by the size field in the header, with the profile flags field (bytes 44 to 47, see 7.2.11), rendering intent field (bytes 64 to 67, see 7.2.15), and profile ID field (bytes 84 to 99) in the profile header temporarily set to zeros (00h), shall be used to calculate the ID. A profile ID field value of zero (00h) shall indicate that a profile ID has not been calculated.

That is, device attributes unique to the profile should contribute to the calculated ID and profile flags that can change depending on how the profile may have been embedded shouldn't. Therefore, shouldn't the code in cmsmd5.c be:

    Icc ->flags = 0;

rather than

    memset(&Icc ->attributes, 0, sizeof(Icc ->attributes));

?

mm2 commented 4 months ago

Thanks for pointing out. I checked the actual spec and you are right. I was wondering how this could have been there for so many years unnoticed, so something ringed a bell in my head. After checking the old spec 4.0.0 (ICC1-V40.pdf, ICC.1:2001-12) that is what it says:

6.1.13 Profile ID Profile ID is generated by using the MD5 fingerprinting method (details of which can be found via the Technical Notes page on the ICC web-site - www.color.org).. The entire profile, based on the size field in the header, is used to calculate the ID with the exception of the Rendering intent, Header attributes, and Profile ID fields in the profile header for which zeros are used instead of their actual values. A Profile ID value of zero indicates the ID has not been calculated. However, it is strongly recommended that profile creators do calculate and fill in the correct value in the Profile ID field

Which is absolutely amazing. ICC changed the spec and I cannot find any notes of this change in the errata. I am modifying the code to reflect this change, wondering how many profiles I will break in the process.