std/jpeg: ignore too-short COM or APPn markers
diff --git a/release/c/wuffs-unsupported-snapshot.c b/release/c/wuffs-unsupported-snapshot.c
index 91cb81e..4cc7e77 100644
--- a/release/c/wuffs-unsupported-snapshot.c
+++ b/release/c/wuffs-unsupported-snapshot.c
@@ -49070,11 +49070,14 @@
}
self->private_impl.f_payload_length = t_4;
}
- if (self->private_impl.f_payload_length < 2u) {
+ if (self->private_impl.f_payload_length >= 2u) {
+ self->private_impl.f_payload_length -= 2u;
+ } else if ((v_marker == 254u) || ((224u <= v_marker) && (v_marker < 240u))) {
+ continue;
+ } else {
status = wuffs_base__make_status(wuffs_jpeg__error__bad_marker);
goto exit;
}
- self->private_impl.f_payload_length -= 2u;
}
if (v_marker < 192u) {
status = wuffs_base__make_status(wuffs_jpeg__error__unsupported_marker);
diff --git a/script/print-jpeg-markers.go b/script/print-jpeg-markers.go
index 7e5665f..a380326 100644
--- a/script/print-jpeg-markers.go
+++ b/script/print-jpeg-markers.go
@@ -86,7 +86,7 @@
}
func posError(pos int, lenSrc int) error {
- return fmt.Errorf("bad JPEG, pos = 0x%08X = %10d, len(src) is 0x%08X = %10d",
+ return fmt.Errorf("bad JPEG, pos = 0x%08X = %10d, len(src) = 0x%08X = %10d",
pos, pos, lenSrc, lenSrc)
}
diff --git a/std/jpeg/decode_jpeg.wuffs b/std/jpeg/decode_jpeg.wuffs
index 872fa29..0619fdf 100644
--- a/std/jpeg/decode_jpeg.wuffs
+++ b/std/jpeg/decode_jpeg.wuffs
@@ -426,10 +426,18 @@
} else {
this.payload_length = args.src.read_u16be_as_u32?()
// Payload length includes the 2 bytes for the u16be just read.
- if this.payload_length < 2 {
+ if this.payload_length >= 2 {
+ this.payload_length -= 2
+ } else if (marker == 0xFE) or // COM.
+ ((0xE0 <= marker) and (marker < 0xF0)) { // APPn.
+ // Strictly speaking, this is invalid according to Section
+ // B.1.1.4 "Marker segments". However, libjpeg-turbo allows it
+ // (for COM and APPn markers; see its jdmarker.c file's
+ // skip_variable function) so we do too.
+ continue
+ } else {
return "#bad marker"
}
- this.payload_length -= 2
}
// Switch on the marker.
diff --git a/test/data/artificial-jpeg/hippopotamus-bad-comment-length.jpeg b/test/data/artificial-jpeg/hippopotamus-bad-comment-length.jpeg
new file mode 100644
index 0000000..e8ae5ac
--- /dev/null
+++ b/test/data/artificial-jpeg/hippopotamus-bad-comment-length.jpeg
Binary files differ
diff --git a/test/data/artificial-jpeg/make-hippopotamus-bad-comment-length.go b/test/data/artificial-jpeg/make-hippopotamus-bad-comment-length.go
new file mode 100644
index 0000000..5a5e1ea
--- /dev/null
+++ b/test/data/artificial-jpeg/make-hippopotamus-bad-comment-length.go
@@ -0,0 +1,77 @@
+// Copyright 2024 The Wuffs Authors.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// https://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or https://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+//
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+
+//go:build ignore
+// +build ignore
+
+package main
+
+// Usage: go run make-hippopotamus-bad-comment-length.go
+//
+// This program inserts a four-byte "FF FE 00 00" fragment into the
+// test/data/hippopotamus.jpeg file. The "FF FE" is a COM (comment) marker. The
+// "00 00" payload length is technically invalid (the minimum length is 2, per
+// the itu-t81.pdf spec section B.1.1.4 "Marker segments") but libjpeg-turbo is
+// more lenient and allows less-than-2 (for COM and APPn markers).
+//
+// https://www.w3.org/Graphics/JPEG/itu-t81.pdf
+//
+// https://github.com/libjpeg-turbo/libjpeg-turbo/blob/2192560d74e6e6cf99dd05928885573be00a8208/jdmarker.c#L869-L876
+//
+// $ go run script/print-jpeg-markers.go test/data/hippopotamus.jpeg
+// pos = 0x00000000 = 0 marker = 0xFF 0xD8 SOI
+// pos = 0x00000002 = 2 marker = 0xFF 0xE0 APP0
+// pos = 0x00000014 = 20 marker = 0xFF 0xDB DQT
+// pos = 0x00000059 = 89 marker = 0xFF 0xDB DQT
+// pos = 0x0000009E = 158 marker = 0xFF 0xC0 SOF0 (Sequential/Baseline)
+// pos = 0x000000B1 = 177 marker = 0xFF 0xC4 DHT
+// pos = 0x000000CC = 204 marker = 0xFF 0xC4 DHT
+// pos = 0x000000FC = 252 marker = 0xFF 0xC4 DHT
+// pos = 0x00000119 = 281 marker = 0xFF 0xC4 DHT
+// pos = 0x0000014B = 331 marker = 0xFF 0xDA SOS
+// pos = 0x000004C1 = 1217 marker = 0xFF 0xD9 EOI
+//
+// $ go run script/print-jpeg-markers.go test/data/artificial-jpeg/hippopotamus-bad-comment-length.jpeg
+// pos = 0x00000000 = 0 marker = 0xFF 0xD8 SOI
+// pos = 0x00000002 = 2 marker = 0xFF 0xE0 APP0
+// pos = 0x00000014 = 20 marker = 0xFF 0xDB DQT
+// pos = 0x00000059 = 89 marker = 0xFF 0xFE COM
+// bad JPEG, pos = 0x0000005B = 91, len(src) = 0x000004C7 = 1223
+
+import (
+ "fmt"
+ "os"
+)
+
+func main() {
+ if err := main1(); err != nil {
+ os.Stderr.WriteString(err.Error() + "\n")
+ os.Exit(1)
+ }
+}
+
+func main1() error {
+ src, err := os.ReadFile("../hippopotamus.jpeg")
+ if err != nil {
+ return err
+ } else if len(src) != 0x4C3 {
+ return fmt.Errorf("bad input length: 0x%X", len(src))
+ }
+
+ part0 := src[0x000:0x059]
+ part1 := src[0x059:0x4C3]
+
+ dst := []byte(nil)
+ dst = append(dst, part0...)
+ dst = append(dst, 0xFF, 0xFE, 0x00, 0x00)
+ dst = append(dst, part1...)
+
+ return os.WriteFile("hippopotamus-bad-comment-length.jpeg", dst, 0666)
+}
diff --git a/test/nia-checksums-of-data.txt b/test/nia-checksums-of-data.txt
index 02412ae..60765ac 100644
--- a/test/nia-checksums-of-data.txt
+++ b/test/nia-checksums-of-data.txt
@@ -21,6 +21,7 @@
OK. 39a71dce test/data/artificial-gif/small-frame-interlaced.gif
OK. beaec397 test/data/artificial-gif/transparent-index.gif
BAD f33b9bc1 test/data/artificial-gif/zero-width-frame.gif
+OK. 96bdbbb3 test/data/artificial-jpeg/hippopotamus-bad-comment-length.jpeg
OK. 96bdbbb3 test/data/artificial-jpeg/hippopotamus-sof-dht-swap.jpeg
OK. 0564b364 test/data/artificial-png/apng-skip-idat.png
OK. e08a7cc8 test/data/artificial-png/exif.png
