An example of XMP metadata that triggers the error "Error writing PNG: XMP metadata contains an invalid null character" in avifpng.c

[wantehchang]

Yannis: Please take a look at the XMP metadata in this image and see if avifpng.c is rejecting it correctly.

Steps to reproduce:

1. Download moon.jpg from https://crbug.com/1405727.
2. avifenc moon.jpg moon.avif
3. avifdec moon.avif moon.png

The avifdec command fails with the error message:

Error writing PNG: XMP metadata contains an invalid null character

[wantehchang]

Running avifdec in the debugger, I found that the null character is at the end of the avif->xmp.data buffer.

[wantehchang]

The XMP specification says "When XMP is encoded as UTF-8, there are no zero bytes in the XMP Packet." It seems that the XMP metadata in moon.jpg has an XMP packet that is incorrectly null-terminated.

[y-guyon]

Error writing PNG: XMP metadata contains an invalid null character

comes from

libavif/apps/shared/avifpng.c

Lines 481 to 485 in d48836b

	// The iTXt XMP payload may not contain a zero byte according to section 4.2.3.3 of
	// the PNG specification, version 1.2.
	if (memchr(avif->xmp.data, '\0', avif->xmp.size)) {
	fprintf(stderr, "Error writing PNG: XMP metadata contains an invalid null character\n");
	goto cleanup;

I assume the issue is that avifenc can generate a file that avifdec refuses. Solutions I can think of:

• Truncate the XMP chunk during PNG import when it ends by a null character (suggested solution)
• Truncate the XMP chunk during PNG export when it ends by a null character
• Truncate the XMP chunk during AVIF export when it ends by a null character
• Remove the XMP chunk during PNG import when it contains a null character
• Remove the XMP chunk during PNG export when it contains a null character
• Remove the XMP chunk during AVIF export when it contains a null character
• Replace null characters in XMP chunk by spaces or newlines or some other delimiter during AVIF export
• Remove the check during PNG export
• Add the check during PNG import

Opinions?

[kmilos]

IMHO, it makes sense to try and sanitize the XMP chunk at the point of import, from any format (this is not tied to PNG spec only, this is in the XMP spec), so you don't even produce an "invalid" AVIF in the first place. I think this is the approach we took in exiv2 as well as libexpat was throwing a similar error and refused to parse it from a sample JPEG...

[y-guyon]

@kmilos Thanks for the info. Do you have any pointer to exiv2 or libexpat code implementing what you described?

[kmilos]

Fond it - take a look at Exiv2/exiv2#2133 and the related issue report.

libexpat just assumes its input is not terminated and simply barfs if it is, it has no workarounds for it. I remember even looking into to the libexpat code to convince myself that the nul character was explicitly in the list of invalid XML characters they check for...

[wantehchang]

We need to first make sure the terminating null byte comes from moon.jpg rather than our code in avifJPEGRead(). That is, we should verify that libjpeg does not null-terminate the data in marker->data and include the appended null byte in marker->data_length. I believe this is the case, but it would be good for another person to doublecheck it.

Once we know avifJPEGRead() is correct, then we change it to handle this kind of XMP metadata error. We can either report this error or fix up this error. If we fix up this error, I suggest we only strip the null byte at the end because it is very likely a programming error. If a null byte occurs in the middle of an XMP packet, that is more likely corrupted data and we should not try to fix that up.

We can leave the check during PNG export unchanged.

[kmilos]

This has nothing to do w/ libjpeg per se, who knows how that original file was created....

[jzern]

As noted, exiv2 (0.27.5) fails to parse the file:

$ exiv2 -p x print moon.jpg 
Error: XMP Toolkit error 201: Error in XMLValidator
Warning: Failed to decode XMP metadata.

exempi (version 2.6.3) will dump the packet, however.

$ exempi -x moon.jpg 
dump_xmp for file moon.jpg
<x:xmpmeta xmlns:x="adobe:ns:meta/" x:xmptk="Exempi + XMP Core 6.0.0">
 <rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
  <rdf:Description rdf:about=""
...

[kmilos]

As noted, exiv2 (0.27.5) fails to parse the file:

Yep, the fix is in exiv2 0.27.6

Btw, exiftool -v5 can also provide insight into all of the chunks for most formats, and for moon.jpg I do see the culprit in the file:

JPEG APP1 (3629 bytes):
    011c: 68 74 74 70 3a 2f 2f 6e 73 2e 61 64 6f 62 65 2e [http://ns.adobe.]
    012c: 63 6f 6d 2f 78 61 70 2f 31 2e 30 2f 00 3c 3f 78 [com/xap/1.0/.<?x]
    013c: 70 61 63 6b 65 74 20 62 65 67 69 6e 3d 22 ef bb [packet begin="..]
    [snip]
    0f2c: 20 20 20 20 20 20 20 20 20 3c 3f 78 70 61 63 6b [         <?xpack]
    0f3c: 65 74 20 65 6e 64 3d 22 77 22 3f 3e 00          [et end="w"?>.]

[y-guyon]

We need to first make sure the terminating null byte comes from moon.jpg rather than our code in avifJPEGRead(). That is, we should verify that libjpeg does not null-terminate the data in marker->data and include the appended null byte in marker->data_length. I believe this is the case, but it would be good for another person to doublecheck it.

I confirm that at the following point:

libavif/apps/shared/avifjpeg.c

Line 532 in d48836b

avifImageSetMetadataXMP(avif, standardXMPData, standardXMPSize);

standardXMPData[standardXMPSize-1] is 0 when reading moon.jpg and 62 when reading paris_exif_xmp_icc.jpg.

[wantehchang]

Miloš, Yannis: Thank you very much for confirming our code is correct and the error is in the moon.jpg file.

Should we report an error if an XMP packet is not valid UTF-8? Or just check if the XMP packet contains a zero byte (similar to the check in avifpng.c)?

Our error message can point to tools that can be used to inspect and correct the errors in the XMP packet.

[y-guyon]

The XMP specification says "When XMP is encoded as UTF-8, there are no zero bytes in the XMP Packet."

I found that sentence only in XMP spec Part 3 section 1.1.2 GIF (Graphic Interchange Format). Does it apply to any XMP chunk in any format (including JPEG)? The PNG spec forbids it too, I did not check other formats.

Should we report an error if an XMP packet is not valid UTF-8? Or just check if the XMP packet contains a zero byte (similar to the check in avifpng.c)?

I started writing a change involving a new avifFixImageXMP() in avifutil.h, that is called from avifJPEGRead() and avifPNGRead(). It just trims trailing \0s. It could also be called during AVIF decoding.
If \0 is forbidden in XMP chunks in JPEG and in AVIF, we can add a check at AVIF encoding. Otherwise, I suggest we just leave it like that: carry any non-trailing \0 over to AVIF and JPEG exports.

[kmilos]

I started writing a change involving a new avifFixImageXMP() in avifutil.h, that is called from avifJPEGRead() and avifPNGRead().

I feel you should do it at a single/common entry point (e.g. avifImageSetMetadataXMP() or some such), not for every possible image format (easy to forget when new ones are added)...

If \0 is forbidden in XMP chunks in JPEG and in AVIF,

It is forbidden in UTF-8 XML period, nothing to do w/ images. As I said, libexpat just flatly refuses to parse null-terminated XML, no matter where it comes from. From https://en.wikipedia.org/wiki/Valid_characters_in_XML:

Note that the code point U+0000, assigned to the null control character, is the only character encoded in Unicode and ISO/IEC 10646 that is always invalid in any XML 1.0 and 1.1 document.