Skip to content

Commit 593d489

Browse files
cowtowncodercowtowncoder
committed
Fixed #263 
1 parent 5740026 commit 593d489

File tree

6 files changed

+222
-10
lines changed

6 files changed

+222
-10
lines changed

‎release-notes/CREDITS-2.x

+2
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,8 @@ Fabian Meumertzheim (fmeum@github)
176176
(2.12.3)
177177
* Reported #261 (cbor) CBORParser need to validate zero-length byte[] for BigInteger
178178
(2.12.3)
179+
* Reported #263 (smile) Handle invalid chunked-binary-format length gracefully
180+
(2.12.3)
179181

180182
(jhhladky@github)
181183

‎release-notes/VERSION-2.x

+2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ Modules:
2222
(reported by Fabian M)
2323
#261 (cbor) CBORParser need to validate zero-length byte[] for BigInteger
2424
(reported by Fabian M)
25+
#263 (smile) Handle invalid chunked-binary-format length gracefully
26+
(reported by Fabian M)
2527

2628
2.12.2 (03-Mar-2021)
2729

‎smile/src/main/java/com/fasterxml/jackson/dataformat/smile/SmileParser.java

+85-7
Original file line numberDiff line numberDiff line change
@@ -2162,20 +2162,98 @@ private final void _finishBigDecimal() throws IOException
21622162
_numberType = NumberType.BIG_DECIMAL;
21632163
}
21642164

2165-
private final int _readUnsignedVInt() throws IOException
2165+
protected final int _readUnsignedVInt() throws IOException
2166+
{
2167+
// 23-Mar-2021, tatu: Let's optimize a bit here: if we have 5 bytes
2168+
// available, can avoid further boundary checks
2169+
if ((_inputPtr + 5) > _inputEnd) {
2170+
return _readUnsignedVIntSlow();
2171+
}
2172+
2173+
int ch = _inputBuffer[_inputPtr++];
2174+
if (ch < 0) {
2175+
return ch & 0x3F;
2176+
}
2177+
int value = ch;
2178+
2179+
// 2nd byte
2180+
ch = _inputBuffer[_inputPtr++];
2181+
if (ch < 0) {
2182+
return (value << 6) + (ch & 0x3F);
2183+
}
2184+
value = (value << 7) + ch;
2185+
2186+
// 3rd byte
2187+
ch = _inputBuffer[_inputPtr++];
2188+
if (ch < 0) {
2189+
return (value << 6) + (ch & 0x3F);
2190+
}
2191+
value = (value << 7) + ch;
2192+
2193+
// 4th byte
2194+
ch = _inputBuffer[_inputPtr++];
2195+
if (ch < 0) {
2196+
return (value << 6) + (ch & 0x3F);
2197+
}
2198+
value = (value << 7) + ch;
2199+
2200+
// 5th byte
2201+
ch = _inputBuffer[_inputPtr++];
2202+
if ((ch >= 0) // invalid, should end
2203+
// Must validate no overflow, as well. We can have at most 31 bits
2204+
// for unsigned int, but with 4 x 7 + 6 == 34 we could have 3 "extra" bits;
2205+
// at this point we have accumulated 28 bits, so shifting right by 25 should
2206+
// not leave any 1 bits left:
2207+
|| ((value >>> 25) != 0) // overflow in first byte
2208+
) {
2209+
_reportInvalidUnsignedVInt(value >>> 21, ch);
2210+
}
2211+
return (value << 6) + (ch & 0x3F);
2212+
}
2213+
2214+
// @since 2.12.3
2215+
protected final int _readUnsignedVIntSlow() throws IOException
21662216
{
21672217
int value = 0;
2168-
while (true) {
2218+
int count = 0;
2219+
2220+
// Read first 4 bytes
2221+
do {
21692222
if (_inputPtr >= _inputEnd) {
21702223
_loadMoreGuaranteed();
21712224
}
2172-
int i = _inputBuffer[_inputPtr++];
2173-
if (i < 0) { // last byte
2174-
value = (value << 6) + (i & 0x3F);
2225+
int ch = _inputBuffer[_inputPtr++];
2226+
if (ch < 0) { // last byte
2227+
value = (value << 6) + (ch & 0x3F);
21752228
return value;
21762229
}
2177-
value = (value << 7) + i;
2230+
value = (value << 7) + ch;
2231+
} while (++count < 4);
2232+
2233+
// but if we need fifth, require validation
2234+
if (_inputPtr >= _inputEnd) {
2235+
_loadMoreGuaranteed();
2236+
}
2237+
int ch = _inputBuffer[_inputPtr++];
2238+
// same validation as in optimized cvase
2239+
if ((ch >= 0) // invalid, did not end with high-bit set
2240+
|| ((value >>> 25) != 0) // overflow in first byte
2241+
) {
2242+
_reportInvalidUnsignedVInt(value >>> 21, ch);
2243+
}
2244+
return (value << 6) + (ch & 0x3F);
2245+
}
2246+
2247+
protected final void _reportInvalidUnsignedVInt(int firstCh, int lastCh) throws IOException
2248+
{
2249+
if (lastCh >= 0) {
2250+
_reportError(
2251+
"Overflow in VInt (current token %s): 5th byte (0x%2X) of 5-byte sequence must have its highest bit set to indicate end",
2252+
currentToken(), lastCh);
21782253
}
2254+
_reportError(
2255+
"Overflow in VInt (current token %s): 1st byte (0x%2X) of 5-byte sequence must have its top 4 bits zeroes",
2256+
currentToken(), firstCh);
21792257
}
21802258

21812259
private final byte[] _read7BitBinaryWithLength() throws IOException
@@ -2740,7 +2818,7 @@ protected void _reportInvalidOther(int mask, int ptr) throws JsonParseException
27402818
protected void _reportIncompleteBinaryRead(int expLen, int actLen) throws IOException
27412819
{
27422820
_reportInvalidEOF(String.format(" for Binary value: expected %d bytes, only found %d",
2743-
expLen, actLen), _currToken);
2821+
expLen, actLen), currentToken());
27442822
}
27452823

27462824
/*

‎smile/src/test/java/com/fasterxml/jackson/dataformat/smile/ParserInternalsTest.java

+93
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
package com.fasterxml.jackson.dataformat.smile;
2+
3+
import java.io.InputStream;
4+
5+
import com.fasterxml.jackson.core.exc.StreamReadException;
6+
import com.fasterxml.jackson.core.io.IOContext;
7+
import com.fasterxml.jackson.core.sym.ByteQuadsCanonicalizer;
8+
import com.fasterxml.jackson.dataformat.smile.testutil.ThrottledInputStream;
9+
10+
public class ParserInternalsTest extends BaseTestForSmile
11+
{
12+
private final ByteQuadsCanonicalizer ROOT_SYMBOLS = ByteQuadsCanonicalizer.createRoot();
13+
14+
public void testPositiveVIntGoodCases() throws Exception
15+
{
16+
// First, couple of known good cases
17+
_verifyVIntOk(0, new byte[] { (byte) 0x80 });
18+
_verifyVIntOk(3, new byte[] { (byte) 0x83 });
19+
_verifyVIntOk(Integer.MAX_VALUE, new byte[] {
20+
0x0F, 0x7F, 0x7F, 0x7F, (byte) 0xBF
21+
});
22+
}
23+
24+
public void testPositiveVIntOverflows() throws Exception
25+
{
26+
// Bad: ends at 5th, but overflows
27+
_verifyVIntBad(new byte[] {
28+
(byte) 0x7F, 0x7F, 0x7F, 0x7F, (byte) 0xBF
29+
}, "Overflow in VInt", "1st byte (0x7F)"
30+
);
31+
// Bad: does not end at 5th (and overflows that way)
32+
_verifyVIntBad(new byte[] {
33+
(byte) 0x7F, 0x7F, 0x7F, 0x7F, (byte) 0x7F
34+
}, "Overflow in VInt", "5th byte (0x7F)"
35+
);
36+
}
37+
38+
private void _verifyVIntOk(int expValue, byte[] doc) throws Exception
39+
{
40+
// First, read with no boundaries
41+
try (SmileParser p = _minimalParser(doc)) {
42+
assertEquals(expValue, p._readUnsignedVInt());
43+
}
44+
45+
// Then incremental read
46+
try (SmileParser p = _minimalParser(new ThrottledInputStream(doc, 1))) {
47+
assertEquals(expValue, p._readUnsignedVInt());
48+
}
49+
}
50+
51+
private void _verifyVIntBad(byte[] doc, String... msgs) throws Exception
52+
{
53+
// First, read with no boundaries
54+
try (SmileParser p = _minimalParser(doc)) {
55+
try {
56+
int val = p._readUnsignedVInt();
57+
fail("Should not pass, got 0x"+Integer.toHexString(val));
58+
} catch (StreamReadException e) {
59+
verifyException(e, msgs);
60+
}
61+
}
62+
63+
// Then incremental read
64+
try (SmileParser p = _minimalParser(new ThrottledInputStream(doc, 1))) {
65+
try {
66+
int val = p._readUnsignedVInt();
67+
fail("Should not pass, got 0x"+Integer.toHexString(val));
68+
} catch (StreamReadException e) {
69+
verifyException(e, msgs);
70+
}
71+
}
72+
}
73+
74+
private SmileParser _minimalParser(byte[] doc) {
75+
IOContext ctxt = new IOContext(null, doc, false);
76+
return new SmileParser(ctxt, // IOContext
77+
0, 0, // flags
78+
null, // (codec)
79+
ROOT_SYMBOLS.makeChild(0), // ByteQuadsCanonicalizer
80+
null, // InputStream
81+
doc, 0, doc.length, false);
82+
}
83+
84+
private SmileParser _minimalParser(InputStream in) {
85+
IOContext ctxt = new IOContext(null, in, false);
86+
return new SmileParser(ctxt, // IOContext
87+
0, 0, // flags
88+
null, // (codec)
89+
ROOT_SYMBOLS.makeChild(0), // ByteQuadsCanonicalizer
90+
in, // InputStream
91+
new byte[1], 0, 0, false);
92+
}
93+
}

‎smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz32180BinaryTest.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,12 @@
88

99
import com.fasterxml.jackson.dataformat.smile.BaseTestForSmile;
1010

11-
// For [dataformats-binary#]
11+
// For [dataformats-binary#260]
1212
public class Fuzz32180BinaryTest extends BaseTestForSmile
1313
{
1414
private final ObjectMapper MAPPER = smileMapper();
1515

16-
// Payload:
17-
public void testInvalidBinary() throws Exception
16+
public void testInvalidRawBinary() throws Exception
1817
{
1918
final byte[] input0 = new byte[] {
2019
0x3A, 0x29, 0x0A, 0x00, // smile signature

‎smile/src/test/java/com/fasterxml/jackson/dataformat/smile/fuzz/Fuzz32339_7BitBinaryTest.java

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package com.fasterxml.jackson.dataformat.smile.fuzz;
2+
3+
import java.io.ByteArrayInputStream;
4+
import java.util.Arrays;
5+
6+
import com.fasterxml.jackson.core.exc.StreamReadException;
7+
import com.fasterxml.jackson.databind.ObjectMapper;
8+
import com.fasterxml.jackson.dataformat.smile.BaseTestForSmile;
9+
10+
//For [dataformats-binary#263]
11+
public class Fuzz32339_7BitBinaryTest extends BaseTestForSmile
12+
{
13+
private final ObjectMapper MAPPER = smileMapper();
14+
15+
// Payload:
16+
public void testInvalid7BitBinary() throws Exception
17+
{
18+
final byte[] input0 = new byte[] {
19+
0x3A, 0x29, 0x0A, 0x00, // smile signature
20+
(byte) 0xE8, // binary, 7-bit encoded
21+
0x35, 0x20, 0x20,
22+
0x20, (byte) 0xFF // 5 byte VInt for 0x7fe4083f (close to Integer.MAX_VALUE)
23+
};
24+
25+
// Let's expand slightly to avoid too early detection, ensure that we are
26+
// not only checking completely missing payload or such
27+
final byte[] input = Arrays.copyOf(input0, 65 * 1024);
28+
29+
try (ByteArrayInputStream bytes = new ByteArrayInputStream(input)) {
30+
try {
31+
/*JsonNode root =*/ MAPPER.readTree(bytes);
32+
} catch (StreamReadException e) {
33+
verifyException(e, "Overflow in VInt (current token VALUE_EMBEDDED_OBJECT");
34+
verifyException(e, "1st byte (0x35)");
35+
}
36+
}
37+
}
38+
}

0 commit comments

Comments
 (0)