False negative: `HandlerBox` (`hdlr`) `name` field shall be null-terminated

[baumanj]

Per ISOBMFF (ISO/IEC 14496-12:2020) § 4.2.1, fields of type utf8string are

UTF-8 string as defined in IETF RFC 3629, null-terminated.

See this AVIF sample from https://github.com/image-rs/image/issues/1504. The hdlr box looks like:

$ hexdump -C -s 32 -n 32 /private/tmp/SampleJPGImage_500kbmb/SampleJPGImage_500kbmb.avif
00000020  00 00 00 20 68 64 6c 72  00 00 00 00 00 00 00 00  |... hdlr........|
00000030  70 69 63 74 00 00 00 00  00 00 00 00 00 00 00 00  |pict............|

Interpreted according to (ISO/IEC 14496-12:2020) § 8.4.3.2:

aligned(8) class HandlerBox extends FullBox('hdlr', version = 0, 0) {
    unsigned int(32) pre_defined = 0;
    unsigned int(32) handler_type;
    const unsigned int(32)[3] reserved = 0;
    utf8string name;
}

we get

00 00 00 20 // Box size (32 bytes)
68 64 6c 72 // boxtype ("hdlr")
00 00 00 00 // version (1 byte), flags (3 bytes)
00 00 00 00 // pre_defined = 0
70 69 63 74 // handler_type ("pict")
00 00 00 00 // reserved[0] = 0
00 00 00 00 // reserved[1] = 0
00 00 00 00 // reserved[2] = 0

There should be an additional 0x00 byte to represent the null-terminated name, but ComplianceWarden doesn't give any error or warning for this.

[baumanj]

I think https://github.com/kornelski/cavif-rs/issues/37 is seeing a false positive on null termination along with the true positive on https://github.com/gpac/ComplianceWarden/issues/29

[rbouqueau]

@baumanj It seems the link to the file expired. Could you share it with me? Github accepts attachments in case it can help.

[baumanj]

Here's a pretty minimal example that reproduces the issue: hdlr-nonzero-reserved.avif.zip

[rbouqueau]

Thanks. The report seems ok:

+--------------------------------------+
|           avif validation            |
+--------------------------------------+

Specification description: AVIF v1.0.0, 19 February 2019
https://aomediacodec.github.io/av1-avif/

========================================
[avif] No errors.
========================================

+--------------------------------------+
|           miaf validation            |
+--------------------------------------+

Specification description: MIAF (Multi-Image Application Format)
MPEG-A part 22 - ISO/IEC 23000-22 - w18260 FDIS - Jan 2019

========================================
[miaf] No errors.
========================================

+--------------------------------------+
|           heif validation            |
+--------------------------------------+

Specification description: HEIF - ISO/IEC 23008-12 - 2nd Edition N18310

========================================
[heif] No errors.
========================================

+--------------------------------------+
|          isobmff validation          |
+--------------------------------------+

Specification description: ISO Base Media File Format
MPEG-4 part 12 - ISO/IEC 14496-12 - m17277 (6th+FDAM1+FDAM2+COR1-R4)

[isobmff][Rule #13] Error: 'hdlr box': reserved1 shall be 0 but value is 1383428980

========================================
[isobmff] 1 error(s), 0 warning(s).
========================================

===== Involved rules descriptions:

[isobmff][Rule #13] Section 8.4.3.1
'hdlr box': pre_defined = 0, reserved = 0, 'name' field shall be null-terminated.

What could I improve here?

[baumanj]

[isobmff][Rule #13] Section 8.4.3.1
'hdlr box': pre_defined = 0, reserved = 0, 'name' field shall be null-terminated.

What could I improve here?

The null-terminated error reported here is a false positive. The name field of the hdlr box for this input is indeed null-terminated, just zero length:

$ hexdump -C -s 44 -n 33 hdlr-nonzero-reserved.avif 
0000002c  00 00 00 21 68 64 6c 72  00 00 00 00 00 00 00 00  |...!hdlr........|
0000003c  70 69 63 74 52 75 73 74  00 00 00 00 00 00 00 00  |pictRust........|
0000004c  00                                                |.|
0000004d

aligned(8) class HandlerBox extends FullBox('hdlr', version = 0, 0) {
    unsigned int(32) pre_defined = 0;
    unsigned int(32) handler_type;
    const unsigned int(32)[3] reserved = 0;
    utf8string name;
}

00 00 00 21: size: 33
68 64 6c 72: type: "hdlr"
00 00 00 00: version: 0, flags: 0, 
00 00 00 00: pre_defined: 0
70 69 63 74: handler_type: "pict"
52 75 73 74: reserved[0]: "Rust" (invalid, correctly reported)
00 00 00 00: reserved[1]: 0
00 00 00 00: reserved[2]: 0
00         : name: ""

[cconcolato]

@baumanj I agree with @rbouqueau . The report is good. There is no false positive.

CW reports in 2 steps: Step 1 - it reports the actual error with a reference to a Rule: [isobmff][Rule #13] Error: 'hdlr box': reserved1 shall be 0 but value is 1383428980

Step 2 - it lists the rule:

[isobmff][Rule #13] Section 8.4.3.1
'hdlr box': pre_defined = 0, reserved = 0, 'name' field shall be null-terminated.

What may be confusing is that the Rule is wide, covering 3 fields (pre_defined, reserved, and name ), but the actual error that goes against the rule is only about the reserved field.

Maybe splitting the rule into 3 rules would make it clearer, but that might be too much work...

[rbouqueau]

I can split that into 3 rules, that's not an issue. @baumanj Would that fix the confusion?

[baumanj]

That sounds like a good idea; I was definitely confused

[rbouqueau]

done and wasm update

Le sam. 4 sept. 2021 à 18:23, baumanj @.***> a écrit :

That sounds like a good idea; I was definitely confused

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gpac/ComplianceWarden/issues/28#issuecomment-913000183, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOUIGFCJ2E3O75HFYDE63DUAJBZXANCNFSM5AHS44QA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.