From 253eaef8c93a28bd0ce72153b6067affb738c0f5 Mon Sep 17 00:00:00 2001 From: Arthur Chan Date: Mon, 18 Dec 2023 11:25:57 +0000 Subject: [PATCH 1/3] Add bound check for array index Signed-off-by: Arthur Chan --- .../jackson/dataformat/smile/SmileParser.java | 3 ++ .../smile/fuzz/Fuzz65126IOOBETest.java | 27 ++++++++++++++++++ .../data/clusterfuzz-smile-65126.smile | Bin 0 -> 151 bytes 3 files changed, 30 insertions(+) create mode 100644 smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz65126IOOBETest.java create mode 100644 smile/src/test/resources/data/clusterfuzz-smile-65126.smile diff --git a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParser.java b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParser.java index c27ce3602..0aaa3b21e 100644 --- a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParser.java +++ b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParser.java @@ -915,6 +915,9 @@ public String nextTextValue() throws IOException } _tokenOffsetForTotal = ptr; // _tokenInputTotal = _currInputProcessed + _inputPtr; + if ((ptr < 0) || (ptr >= _inputBuffer.length)) { + _reportError("Invalid text length"); + } int ch = _inputBuffer[ptr++] & 0xFF; _typeAsInt = ch; diff --git a/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz65126IOOBETest.java b/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz65126IOOBETest.java new file mode 100644 index 000000000..2c2522d8a --- /dev/null +++ b/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz65126IOOBETest.java @@ -0,0 +1,27 @@ +package com.fasterxml.jackson.dataformat.smile.fuzz; + +import com.fasterxml.jackson.core.*; +import com.fasterxml.jackson.core.exc.StreamReadException; +import com.fasterxml.jackson.databind.ObjectMapper; + +import com.fasterxml.jackson.dataformat.smile.BaseTestForSmile; + +public class Fuzz65126IOOBETest extends BaseTestForSmile +{ + private final ObjectMapper MAPPER = smileMapper(); + + // [dataformats-binary#426] + public void testInvalidIOOBE() throws Exception + { + final byte[] input = readResource("/data/clusterfuzz-smile-65126.smile"); + try (JsonParser p = MAPPER.createParser(input)) { + try { + p.getTextOffset(); + p.nextTextValue(); + p.nextTextValue(); + } catch (StreamReadException e) { + verifyException(e, "Invalid text length"); + } + } + } +} diff --git a/smile/src/test/resources/data/clusterfuzz-smile-65126.smile b/smile/src/test/resources/data/clusterfuzz-smile-65126.smile new file mode 100644 index 0000000000000000000000000000000000000000..f02725dc4ddb7ea8e05aac1422d359e86eb8fd3d GIT binary patch literal 151 zcmaFCA7AJ20tx*8&vpR>G@>;$H2wzz2@MmCPZ}#g3_RfCbqx&*4GkTzlBCL#R0V?y uRi*#`|EE?Iq$Zc7rYNYU0C_+#>HmMj{{}`88Xyj;iE}kH3>X*~fPw(85j=PR literal 0 HcmV?d00001 From 4bcf4247494ad6f03e18fca0cb80e09a9cb05d7d Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 18 Dec 2023 20:19:35 -0800 Subject: [PATCH 2/3] Minor tweak to test case --- ...z65126IOOBETest.java => Fuzz_426_65126IOOBETest.java} | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) rename smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/{Fuzz65126IOOBETest.java => Fuzz_426_65126IOOBETest.java} (70%) diff --git a/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz65126IOOBETest.java b/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz_426_65126IOOBETest.java similarity index 70% rename from smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz65126IOOBETest.java rename to smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz_426_65126IOOBETest.java index 2c2522d8a..562ea75fd 100644 --- a/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz65126IOOBETest.java +++ b/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz_426_65126IOOBETest.java @@ -6,7 +6,7 @@ import com.fasterxml.jackson.dataformat.smile.BaseTestForSmile; -public class Fuzz65126IOOBETest extends BaseTestForSmile +public class Fuzz_426_65126IOOBETest extends BaseTestForSmile { private final ObjectMapper MAPPER = smileMapper(); @@ -15,10 +15,13 @@ public void testInvalidIOOBE() throws Exception { final byte[] input = readResource("/data/clusterfuzz-smile-65126.smile"); try (JsonParser p = MAPPER.createParser(input)) { + assertNull(p.nextTextValue()); + assertToken(JsonToken.VALUE_EMBEDDED_OBJECT, p.currentToken()); try { - p.getTextOffset(); - p.nextTextValue(); +// byte[] b = p.getBinaryValue(); +// assertEquals(100, b.length); p.nextTextValue(); + fail("Should not pass"); } catch (StreamReadException e) { verifyException(e, "Invalid text length"); } From 7f4469bb2e2883473f3bf7ed5f4bcf8f437aafc7 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 18 Dec 2023 20:43:51 -0800 Subject: [PATCH 3/3] Update release notes, change fix a bit --- release-notes/CREDITS-2.x | 6 ++++-- release-notes/VERSION-2.x | 6 ++++-- .../jackson/dataformat/smile/SmileParser.java | 17 ++++++++++++++--- .../smile/fuzz/Fuzz_426_65126IOOBETest.java | 4 +--- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 471067b9c..f759e89e2 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -284,6 +284,8 @@ Simon Daudin (@simondaudin) Arthur Chan (@arthurscchan) * Contributed #417: (ion) `IonReader` classes contain assert statement which could throw unexpected `AssertionError` - (2.17.0) + (2.17.0) * Contributed #420: (ion) `IndexOutOfBoundsException` thrown by `IonReader` implementations - (2.17.0) + (2.17.0) + * Contributed #426: (smile) `SmileParser` throws unexpected IOOBE for corrupt content + (2.17.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index bb5a266ca..7913e057b 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -18,10 +18,12 @@ Active maintainers: #417: (ion) `IonReader` classes contain assert statement which could throw unexpected `AssertionError` - (contributed by Arthur C) + (fix contributed by Arthur C) #420: (ion) `IndexOutOfBoundsException` thrown by `IonReader` implementations are not handled - (contributed by Arthur C) + (fix contributed by Arthur C) +#426: (smile) `SmileParser` throws unexpected IOOBE for corrupt content + (fix contributed by Arthur C) -(ion) Update `com.amazon.ion:ion-java` to 1.11.0 (from 1.10.5) 2.16.0 (15-Nov-2023) diff --git a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParser.java b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParser.java index 0aaa3b21e..e65907684 100644 --- a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParser.java +++ b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParser.java @@ -915,9 +915,6 @@ public String nextTextValue() throws IOException } _tokenOffsetForTotal = ptr; // _tokenInputTotal = _currInputProcessed + _inputPtr; - if ((ptr < 0) || (ptr >= _inputBuffer.length)) { - _reportError("Invalid text length"); - } int ch = _inputBuffer[ptr++] & 0xFF; _typeAsInt = ch; @@ -2896,6 +2893,11 @@ protected void _skipIncomplete() throws IOException protected void _skipBytes(int len) throws IOException { + // 18-Dec-2023, tatu: Sanity check related to some OSS-Fuzz findings: + if (len < 0) { + throw _constructReadException("Internal error: _skipBytes() called with negative value: %d", + len); + } while (true) { int toAdd = Math.min(len, _inputEnd - _inputPtr); _inputPtr += toAdd; @@ -2917,6 +2919,15 @@ protected void _skip7BitBinary() throws IOException // Ok; 8 encoded bytes for 7 payload bytes first int chunks = origBytes / 7; int encBytes = chunks * 8; + + // sanity check: not all length markers valid; due to signed int(32) + // calculations maximum length only 7/8 of 2^31 + if (encBytes < 0) { + throw _constructReadException( + "Invalid content: invalid 7-bit binary encoded byte length (0x%X) exceeds maximum valid value", + origBytes); + } + // and for last 0 - 6 bytes, last+1 (except none if no leftovers) origBytes -= 7 * chunks; if (origBytes > 0) { diff --git a/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz_426_65126IOOBETest.java b/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz_426_65126IOOBETest.java index 562ea75fd..f25b23590 100644 --- a/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz_426_65126IOOBETest.java +++ b/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz_426_65126IOOBETest.java @@ -18,12 +18,10 @@ public void testInvalidIOOBE() throws Exception assertNull(p.nextTextValue()); assertToken(JsonToken.VALUE_EMBEDDED_OBJECT, p.currentToken()); try { -// byte[] b = p.getBinaryValue(); -// assertEquals(100, b.length); p.nextTextValue(); fail("Should not pass"); } catch (StreamReadException e) { - verifyException(e, "Invalid text length"); + verifyException(e, "Invalid content: invalid 7-bit binary encoded byte length"); } } }