diff --git a/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java b/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java index 64a424b41..a77842a2b 100644 --- a/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java +++ b/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java @@ -2558,7 +2558,7 @@ protected byte[] _finishLongContiguousBytes(final int expLen) throws IOException int count = Math.min(avail, left); bb.write(_inputBuffer, _inputPtr, count); _inputPtr += count; - left -= count; + left -= count; } return bb.toByteArray(); } diff --git a/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/fuzz/Fuzz32173LongTextTest.java b/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/fuzz/Fuzz32173LongTextTest.java index 3c402a41d..a76761b6a 100644 --- a/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/fuzz/Fuzz32173LongTextTest.java +++ b/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/fuzz/Fuzz32173LongTextTest.java @@ -10,7 +10,7 @@ public class Fuzz32173LongTextTest extends CBORTestBase { private final ObjectMapper MAPPER = cborMapper(); - public void testInvalidShortText() throws Exception + public void testTruncatedLongText() throws Exception { final byte[] input = new byte[] { 0x7A, // Text value diff --git a/pom.xml b/pom.xml index 702a88bd2..507481e2a 100644 --- a/pom.xml +++ b/pom.xml @@ -84,6 +84,10 @@ com/fasterxml/jackson/**/failing/*.java + + -Xmx1024m diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index e3e8402ee..8080ac8f9 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -172,6 +172,8 @@ Fabian Meumertzheim (fmeum@github) * Reported #259: (cbor) Failed to handle case of alleged String with length of Integer.MAX_VALUE (2.12.3) +* Reported #260: (smile) Allocate byte[] lazily for longer Smile binary data payloads + (2.12.3) * Reported #261 (cbor) CBORParser need to validate zero-length byte[] for BigInteger (2.12.3) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index c1f093711..698e92069 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -18,6 +18,8 @@ Modules: (reported by Fabian M) #259: (cbor) Failed to handle case of alleged String with length of Integer.MAX_VALUE (reported by Fabian M) +#260: (smile) Allocate byte[] lazily for longer Smile binary data payloads + (reported by Fabian M) #261 (cbor) CBORParser need to validate zero-length byte[] for BigInteger (reported by Fabian M) 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 9699d6105..0e402e0f8 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 @@ -8,6 +8,7 @@ import com.fasterxml.jackson.core.*; import com.fasterxml.jackson.core.io.IOContext; import com.fasterxml.jackson.core.sym.ByteQuadsCanonicalizer; +import com.fasterxml.jackson.core.util.ByteArrayBuilder; import static com.fasterxml.jackson.dataformat.smile.SmileConstants.BYTE_MARKER_END_OF_STRING; @@ -239,9 +240,11 @@ private final byte _nextByteGuaranteed() throws IOException } protected final void _loadMoreGuaranteed() throws IOException { - if (!_loadMore()) { _reportInvalidEOF(); } + if (!_loadMore()) { + _reportInvalidEOF(); + } } - + protected final boolean _loadMore() throws IOException { //_currInputRowStart -= _inputEnd; @@ -1895,7 +1898,7 @@ protected final void _finishToken() throws IOException _binaryValue = _read7BitBinaryWithLength(); return; case 7: // binary, raw - _finishRawBinary(); + _binaryValue = _finishRawBinary(); return; } } @@ -2403,27 +2406,67 @@ private final void _decodeLongUnicode() throws IOException _textBuffer.setCurrentLength(outPtr); } - private final void _finishRawBinary() throws IOException + private final byte[] _finishRawBinary() throws IOException { int byteLen = _readUnsignedVInt(); - _binaryValue = new byte[byteLen]; + + // 20-Mar-2021, tatu [dataformats-binary#260]: avoid eager allocation + // for very large content + if (byteLen > LONGEST_NON_CHUNKED_BINARY) { + return _finishLongContiguousBytes(byteLen); + } + + final int expLen = byteLen; + final byte[] b = new byte[byteLen]; + if (_inputPtr >= _inputEnd) { + if (!_loadMore()) { + _reportIncompleteBinaryRead(expLen, 0); + } _loadMoreGuaranteed(); } int ptr = 0; while (true) { int toAdd = Math.min(byteLen, _inputEnd - _inputPtr); - System.arraycopy(_inputBuffer, _inputPtr, _binaryValue, ptr, toAdd); + System.arraycopy(_inputBuffer, _inputPtr, b, ptr, toAdd); _inputPtr += toAdd; ptr += toAdd; byteLen -= toAdd; if (byteLen <= 0) { - return; + return b; + } + if (!_loadMore()) { + _reportIncompleteBinaryRead(expLen, ptr); } - _loadMoreGuaranteed(); } } - + + // @since 2.12.3 + protected byte[] _finishLongContiguousBytes(final int expLen) throws IOException + { + int left = expLen; + + // 20-Mar-2021, tatu: Let's NOT use recycled instance since we have much + // longer content and there is likely less benefit of trying to recycle + // segments + try (final ByteArrayBuilder bb = new ByteArrayBuilder(LONGEST_NON_CHUNKED_BINARY >> 1)) { + while (left > 0) { + int avail = _inputEnd - _inputPtr; + if (avail <= 0) { + if (!_loadMore()) { + _reportIncompleteBinaryRead(expLen, expLen-left); + } + avail = _inputEnd - _inputPtr; + } + int count = Math.min(avail, left); + bb.write(_inputBuffer, _inputPtr, count); + _inputPtr += count; + left -= count; + } + return bb.toByteArray(); + } + } + /* /********************************************************** /* Internal methods, skipping @@ -2693,6 +2736,13 @@ protected void _reportInvalidOther(int mask, int ptr) throws JsonParseException _reportInvalidOther(mask); } + // @since 2.12.3 + protected void _reportIncompleteBinaryRead(int expLen, int actLen) throws IOException + { + _reportInvalidEOF(String.format(" for Binary value: expected %d bytes, only found %d", + expLen, actLen), _currToken); + } + /* /********************************************************** /* Internal methods, other diff --git a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java index f31a66c9f..4e0975b4a 100644 --- a/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java +++ b/smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParserBase.java @@ -23,6 +23,10 @@ public abstract class SmileParserBase extends ParserMinimalBase { protected final static String[] NO_STRINGS = new String[0]; + // 2.12.3: [dataformats-binary#260] Avoid OOME/DoS for bigger binary; + // read only up to 250k + protected final static int LONGEST_NON_CHUNKED_BINARY = 250_000; + /* /********************************************************** /* Config diff --git a/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz32180BinaryTest.java b/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz32180BinaryTest.java new file mode 100644 index 000000000..b63031341 --- /dev/null +++ b/smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz32180BinaryTest.java @@ -0,0 +1,43 @@ +package com.fasterxml.jackson.dataformat.smile.fuzz; + +import java.io.ByteArrayInputStream; +import java.util.Arrays; + +import com.fasterxml.jackson.core.io.JsonEOFException; +import com.fasterxml.jackson.databind.ObjectMapper; + +import com.fasterxml.jackson.dataformat.smile.BaseTestForSmile; + +// For [dataformats-binary#] +public class Fuzz32180BinaryTest extends BaseTestForSmile +{ + private final ObjectMapper MAPPER = smileMapper(); + + // Payload: + public void testInvalidBinary() throws Exception + { + final byte[] input0 = new byte[] { + 0x3A, 0x29, 0x0A, 0x00, // smile signature + (byte) 0xFD, // raw binary + 0x0F, 0x7E, 0x20, + 0x20, (byte) 0xFF, // 5 byte VInt for 0x7fe4083f (close to Integer.MAX_VALUE) + // and one byte of binary payload + 0x00 + }; + // Let's expand slightly to avoid too early detection, ensure that we are + // not only checking completely missing payload or such + final byte[] input = Arrays.copyOf(input0, 65 * 1024); + + try (ByteArrayInputStream bytes = new ByteArrayInputStream(input)) { + try { + /*JsonNode root =*/ MAPPER.readTree(bytes); + } catch (JsonEOFException e) { + verifyException(e, "Unexpected end-of-input for Binary value: expected 2145650751 bytes, only found 66550"); + } catch (OutOfMemoryError e) { + // Just to make it easier to see on fail (not ideal but better than nothing) + e.printStackTrace(); + throw e; + } + } + } +}