Skip to content

Commit

Permalink
TOML: check nesting depth (#398)
Browse files Browse the repository at this point in the history
  • Loading branch information
pjfanning authored Mar 7, 2023
1 parent 0389480 commit 5dd5f74
Show file tree
Hide file tree
Showing 16 changed files with 148 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ public class JavaPropsFactory extends JsonFactory
/**********************************************************
*/

public JavaPropsFactory() { }
public JavaPropsFactory() {
super();
}

public JavaPropsFactory(ObjectCodec codec) {
super(codec);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ public static ObjectNode parse(
Parser parser = new Parser(new TomlFactory(), ioContext,
new TomlStreamReadException.ErrorContext(ioContext.contentReference(), null), options, reader);
try {
return parser.parse();
final ObjectNode node = parser.parse();
assert parser.getNestingDepth() == 0;
return node;
} finally {
parser.lexer.releaseBuffers();
}
Expand All @@ -80,12 +82,18 @@ public static ObjectNode parse(
new TomlStreamReadException.ErrorContext(ioContext.contentReference(), null),
factory.getFormatParserFeatures(), reader);
try {
return parser.parse();
final ObjectNode node = parser.parse();
assert parser.getNestingDepth() == 0;
return node;
} finally {
parser.lexer.releaseBuffers();
}
}

int getNestingDepth() {
return lexer.getNestingDepth();
}

private TomlToken peek() throws TomlStreamReadException {
TomlToken here = this.next;
if (here == null) throw errorContext.atPosition(lexer).generic("Premature end of file");
Expand Down Expand Up @@ -376,7 +384,7 @@ private ValueNode parseIntFromBuffer(char[] buffer, int start, int length) throw
}

private JsonNode parseFloat(int nextState) throws IOException {
String text = lexer.yytext().replace("_", "");
final String text = lexer.yytext().replace("_", "");
pollExpected(TomlToken.FLOAT, nextState);
if (text.endsWith("nan")) {
return factory.numberNode(Double.NaN);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public final class TomlFactory extends JsonFactory
*/

public TomlFactory() {
super();
_tomlParserFeatures = DEFAULT_TOML_PARSER_FEATURE_FLAGS;
_tomlGeneratorFeatures = DEFAULT_TOML_GENERATOR_FEATURE_FLAGS;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package com.fasterxml.jackson.dataformat.toml;

%init{
this.ioContext = ioContext;
this.streamReadConstraints = ioContext.streamReadConstraints();
this.errorContext = errorContext;
yybegin(EXPECT_EXPRESSION);
this.zzBuffer = ioContext.allocTokenBuffer();
Expand All @@ -30,6 +31,8 @@ this.textBuffer = ioContext.constructReadConstrainedTextBuffer();

private boolean trimmedNewline;
final com.fasterxml.jackson.core.util.TextBuffer textBuffer;
private final com.fasterxml.jackson.core.StreamReadConstraints streamReadConstraints;
private int nestingDepth;

private void requestLargerBuffer() throws TomlStreamReadException {
if (prohibitInternalBufferAllocate) {
Expand All @@ -54,6 +57,10 @@ this.textBuffer = ioContext.constructReadConstrainedTextBuffer();
textBuffer.releaseBuffers();
}

public int getNestingDepth() {
return nestingDepth;
}

private void startString() {
// resetWithEmpty does not set _currentSegment, so we need this variant to be able to append further data
textBuffer.emptyAndGetCurrentSegment();
Expand Down Expand Up @@ -257,8 +264,14 @@ HexDig = [0-9A-Fa-f]
yybegin(LITERAL_STRING);
startString();
}
{StdTableOpen} {return TomlToken.STD_TABLE_OPEN;}
{ArrayTableOpen} {return TomlToken.ARRAY_TABLE_OPEN;}
{StdTableOpen} {
streamReadConstraints.validateNestingDepth(++nestingDepth);
return TomlToken.STD_TABLE_OPEN;
}
{ArrayTableOpen} {
streamReadConstraints.validateNestingDepth(++nestingDepth);
return TomlToken.ARRAY_TABLE_OPEN;
}
{KeyValSep} {return TomlToken.KEY_VAL_SEP;}
{NewLine} {}
{Comment} {}
Expand All @@ -284,9 +297,18 @@ HexDig = [0-9A-Fa-f]
startString();
}
{KeyValSep} {return TomlToken.KEY_VAL_SEP;}
{InlineTableClose} {return TomlToken.INLINE_TABLE_CLOSE;}
{StdTableClose} {return TomlToken.STD_TABLE_CLOSE;}
{ArrayTableClose} {return TomlToken.ARRAY_TABLE_CLOSE;}
{InlineTableClose} {
nestingDepth--;
return TomlToken.INLINE_TABLE_CLOSE;
}
{StdTableClose} {
nestingDepth--;
return TomlToken.STD_TABLE_CLOSE;
}
{ArrayTableClose} {
nestingDepth--;
return TomlToken.ARRAY_TABLE_CLOSE;
}
}

<EXPECT_EOL> {
Expand Down Expand Up @@ -340,18 +362,30 @@ HexDig = [0-9A-Fa-f]
}

// inline array / table
{ArrayOpen} {WsCommentNewlineNonEmpty}* {return TomlToken.ARRAY_OPEN;}
{InlineTableOpen} {return TomlToken.INLINE_TABLE_OPEN;}
{ArrayOpen} {WsCommentNewlineNonEmpty}* {
streamReadConstraints.validateNestingDepth(++nestingDepth);
return TomlToken.ARRAY_OPEN;
}
{InlineTableOpen} {
streamReadConstraints.validateNestingDepth(++nestingDepth);
return TomlToken.INLINE_TABLE_OPEN;
}

// array end just after comma
{WsCommentNewlineNonEmpty}* {ArrayClose} {return TomlToken.ARRAY_CLOSE;}
{WsCommentNewlineNonEmpty}* {ArrayClose} {
nestingDepth--;
return TomlToken.ARRAY_CLOSE;
}
}

<EXPECT_ARRAY_SEP> {
// array-values = ws-comment-newline val ws-comment-newline array-sep array-values
// array-values =/ ws-comment-newline val ws-comment-newline [ array-sep ]
{Comma} {WsCommentNewlineNonEmpty}* {return TomlToken.COMMA;}
{ArrayClose} {return TomlToken.ARRAY_CLOSE;}
{ArrayClose} {
nestingDepth--;
return TomlToken.ARRAY_CLOSE;
}
{WsCommentNewlineNonEmpty} {} // always allowed here
}

Expand All @@ -360,7 +394,10 @@ HexDig = [0-9A-Fa-f]
// inline-table-keyvals = keyval [ inline-table-sep inline-table-keyvals ]

{Ws} {Comma} {Ws} {return TomlToken.COMMA;}
{InlineTableClose} {return TomlToken.INLINE_TABLE_CLOSE;}
{InlineTableClose} {
nestingDepth--;
return TomlToken.INLINE_TABLE_CLOSE;
}
}

<BASIC_STRING> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

public class BigStringsTest
public class BigStringsTest extends TomlMapperTestBase
{

final static class StringWrapper
Expand All @@ -25,7 +25,7 @@ void setString(String string) {
}
}

private final TomlMapper MAPPER = new TomlMapper();
private final TomlMapper MAPPER = newTomlMapper();

private TomlMapper newMapperWithUnlimitedStringSizeSupport() {
TomlFactory tomlFactory = TomlFactory.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;

// Copied from YAML modules "DatabindAdvancedTest"
public class ComplexPojoReadWriteTest
public class ComplexPojoReadWriteTest extends TomlMapperTestBase
{
enum Size { SMALL, LARGE; }

Expand Down Expand Up @@ -145,7 +145,7 @@ public Image(String uri, String title, int w, int h, Size s)
@Test
public void testReadWriteComplexPojo() throws Exception
{
ObjectMapper mapper = new TomlMapper();
ObjectMapper mapper = newTomlMapper();

MediaItem input = new MediaItem();
MediaContent content = new MediaContent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import java.util.stream.Collectors;

@RunWith(Parameterized.class)
public class ComplianceInvalidTest {
public class ComplianceInvalidTest extends TomlMapperTestBase {
@Parameterized.Parameters
public static Collection<Object[]> data() throws IOException {
Path folder = Paths.get("compliance", "invalid");
Expand All @@ -28,7 +28,7 @@ public static Collection<Object[]> data() throws IOException {
.collect(Collectors.toList());
}

private final ObjectMapper MAPPER = new TomlMapper();
private final ObjectMapper MAPPER = newTomlMapper();

private final Path path;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import java.util.stream.Collectors;

@RunWith(Parameterized.class)
public class ComplianceValidTest {
public class ComplianceValidTest extends TomlMapperTestBase {
@Parameterized.Parameters
public static Collection<Object[]> data() throws IOException {
Path folder = Paths.get("compliance", "valid");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.io.IOException;
import java.util.Arrays;

import com.fasterxml.jackson.core.exc.StreamConstraintsException;
import org.junit.Assert;
import org.junit.Test;

Expand All @@ -13,9 +14,9 @@
/**
* Collection of OSS-Fuzz found issues for TOML format module.
*/
public class FuzzTomlReadTest
public class FuzzTomlReadTest extends TomlMapperTestBase
{
private final ObjectMapper TOML_MAPPER = new TomlMapper();
private final ObjectMapper TOML_MAPPER = newTomlMapper();

// https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=50036
@Test
Expand Down Expand Up @@ -76,6 +77,21 @@ public void testNumberParsingFail50395() throws Exception
verifyException(e, "Premature end of file");
}
}

@Test
public void testStackOverflow50083() throws Exception
{
StringBuilder input = new StringBuilder();
for (int i = 0; i < 9999; i++) {
input.append("a={");
}
try {
TOML_MAPPER.readTree(input.toString());
Assert.fail("Should not pass");
} catch (StreamConstraintsException e) {
verifyException(e, "Depth (1001) exceeds the maximum allowed nesting depth (1000)");
}
}

protected void verifyException(Throwable e, String... matches)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.fasterxml.jackson.dataformat.toml;

import com.fasterxml.jackson.core.StreamReadConstraints;
import com.fasterxml.jackson.core.exc.StreamConstraintsException;
import com.fasterxml.jackson.core.io.ContentReference;
import com.fasterxml.jackson.core.io.IOContext;
import com.fasterxml.jackson.core.util.BufferRecyclers;
Expand All @@ -15,14 +14,14 @@
import java.math.BigDecimal;
import java.math.BigInteger;

public class LongTokenTest {
public class LongTokenTest extends TomlMapperTestBase {
private static final int SCALE = 10000; // must be bigger than the default buffer size

// Need to ensure max-number-limit not hit
private final TomlFactory FACTORY = TomlFactory.builder()
.streamReadConstraints(StreamReadConstraints.builder().maxNumberLength(Integer.MAX_VALUE).build())
.build();
private final ObjectMapper NO_LIMITS_MAPPER = new TomlMapper(FACTORY);
private final ObjectMapper NO_LIMITS_MAPPER = newTomlMapper(FACTORY);

@Test
public void decimal() throws IOException {
Expand All @@ -42,7 +41,7 @@ public void decimal() throws IOException {
@Test
public void decimalTooLong() throws IOException {
// default TomlFactory has max num length of 1000
final ObjectMapper mapper = new TomlMapper();
final ObjectMapper mapper = newTomlMapper();
StringBuilder toml = new StringBuilder("foo = 0.");
for (int i = 0; i < SCALE; i++) {
toml.append('0');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import com.fasterxml.jackson.databind.json.JsonMapper;

// Composed of pieces from other format modules' tests
public class POJOReadWriteTest
public class POJOReadWriteTest extends TomlMapperTestBase
{
@JsonPropertyOrder({ "topLeft", "bottomRight" })
protected static class Rectangle {
Expand Down Expand Up @@ -150,7 +150,7 @@ public int hashCode() {
}
}

private final ObjectMapper MAPPER = new TomlMapper();
private final ObjectMapper MAPPER = newTomlMapper();
private final ObjectMapper JSON_MAPPER = new JsonMapper();

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
import java.util.Arrays;

@SuppressWarnings("OctalInteger")
public class ParserTest {
private static final ObjectMapper TOML_MAPPER = new TomlMapper();
public class ParserTest extends TomlMapperTestBase {
private static final ObjectMapper TOML_MAPPER = newTomlMapper();
private static final ObjectMapper jsonMapper = JsonMapper.builder()
.enable(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)
.enable(JsonReadFeature.ALLOW_NON_NUMERIC_NUMBERS)
Expand All @@ -42,11 +42,11 @@ static ObjectNode toml(@Language("toml") String toml) throws IOException {
return (ObjectNode) TOML_MAPPER.readTree(toml);
}

static ObjectNode toml(int opts, @Language("toml") String toml) throws IOException {
static ObjectNode toml(TomlFactory factory, @Language("toml") String toml) throws IOException {
return Parser.parse(
factory,
new IOContext(BufferRecyclers.getBufferRecycler(),
ContentReference.rawReference(toml), false),
opts,
new StringReader(toml)
);
}
Expand Down Expand Up @@ -950,15 +950,16 @@ public void bigintBase() throws IOException {
@Test
public void javaTimeDeser() throws IOException {
// this is the same test as above, except with explicit java.time deserialization
int options = TomlReadFeature.PARSE_JAVA_TIME.getMask();
final TomlFactory tomlFactory = newTomlFactory();
tomlFactory.enable(TomlReadFeature.PARSE_JAVA_TIME);

Assert.assertEquals(
JsonNodeFactory.instance.objectNode()
.<ObjectNode>set("odt1", JsonNodeFactory.instance.pojoNode(OffsetDateTime.parse("1979-05-27T07:32:00Z")))
.<ObjectNode>set("odt2", JsonNodeFactory.instance.pojoNode(OffsetDateTime.parse("1979-05-27T00:32:00-07:00")))
.<ObjectNode>set("odt3", JsonNodeFactory.instance.pojoNode(OffsetDateTime.parse("1979-05-27T00:32:00.999999-07:00")))
.<ObjectNode>set("odt4", JsonNodeFactory.instance.pojoNode(OffsetDateTime.parse("1979-05-27T07:32:00Z"))),
toml(options,
toml(tomlFactory,
"odt1 = 1979-05-27T07:32:00Z\n" +
"odt2 = 1979-05-27T00:32:00-07:00\n" +
"odt3 = 1979-05-27T00:32:00.999999-07:00\n" +
Expand All @@ -968,20 +969,20 @@ public void javaTimeDeser() throws IOException {
JsonNodeFactory.instance.objectNode()
.<ObjectNode>set("ldt1", JsonNodeFactory.instance.pojoNode(LocalDateTime.parse("1979-05-27T07:32:00")))
.<ObjectNode>set("ldt2", JsonNodeFactory.instance.pojoNode(LocalDateTime.parse("1979-05-27T00:32:00.999999"))),
toml(options,
toml(tomlFactory,
"ldt1 = 1979-05-27T07:32:00\n" +
"ldt2 = 1979-05-27T00:32:00.999999")
);
Assert.assertEquals(
JsonNodeFactory.instance.objectNode()
.set("ld1", JsonNodeFactory.instance.pojoNode(LocalDate.parse("1979-05-27"))),
toml(options, "ld1 = 1979-05-27")
toml(tomlFactory, "ld1 = 1979-05-27")
);
Assert.assertEquals(
JsonNodeFactory.instance.objectNode()
.<ObjectNode>set("lt1", JsonNodeFactory.instance.pojoNode(LocalTime.parse("07:32:00")))
.<ObjectNode>set("lt2", JsonNodeFactory.instance.pojoNode(LocalTime.parse("00:32:00.999999"))),
toml(options,
toml(tomlFactory,
"lt1 = 07:32:00\n" +
"lt2 = 00:32:00.999999")
);
Expand Down
Loading

0 comments on commit 5dd5f74

Please sign in to comment.