Merge branch '518-limit-the-depth-of-nested-bdf-structures' into 'master'

Limit the depth of nested BDF structures

Closes #518

See merge request !375

briar-core/src/org/briarproject/data/BdfReaderImpl.java

@@ -8,6 +8,8 @@ import org.briarproject.api.data.BdfReader;
 import java.io.IOException;
 import java.io.InputStream;
 
+import javax.annotation.concurrent.NotThreadSafe;
+
 import static org.briarproject.api.data.BdfDictionary.NULL_VALUE;
 import static org.briarproject.data.Types.DICTIONARY;
 import static org.briarproject.data.Types.END;
@@ -27,9 +29,11 @@ import static org.briarproject.data.Types.STRING_32;
 import static org.briarproject.data.Types.STRING_8;
 import static org.briarproject.data.Types.TRUE;
 
-// This class is not thread-safe
+@NotThreadSafe
 class BdfReaderImpl implements BdfReader {
 
+	final static int DEFAULT_NESTED_LIMIT = 5;
+
 	private static final byte[] EMPTY_BUFFER = new byte[] {};
 
 	private final InputStream in;
@@ -37,9 +41,16 @@ class BdfReaderImpl implements BdfReader {
 	private boolean hasLookahead = false, eof = false;
 	private byte next;
 	private byte[] buf = new byte[8];
+	private final int nestedLimit;
 
 	BdfReaderImpl(InputStream in) {
 		this.in = in;
+		this.nestedLimit = DEFAULT_NESTED_LIMIT;
+	}
+
+	BdfReaderImpl(InputStream in, int nestedLimit) {
+		this.in = in;
+		this.nestedLimit = nestedLimit;
 	}
 
 	private void readLookahead() throws IOException {
@@ -77,7 +88,7 @@ class BdfReaderImpl implements BdfReader {
 		}
 	}
 
-	private Object readObject() throws IOException {
+	private Object readObject(int level) throws IOException {
 		if (hasNull()) {
 			readNull();
 			return NULL_VALUE;
@@ -87,8 +98,8 @@ class BdfReaderImpl implements BdfReader {
 		if (hasDouble()) return readDouble();
 		if (hasString()) return readString(Integer.MAX_VALUE);
 		if (hasRaw()) return readRaw(Integer.MAX_VALUE);
-		if (hasList()) return readList();
-		if (hasDictionary()) return readDictionary();
+		if (hasList()) return readList(level);
+		if (hasDictionary()) return readDictionary(level);
 		throw new FormatException();
 	}
 
@@ -287,10 +298,15 @@ class BdfReaderImpl implements BdfReader {
 	}
 
 	public BdfList readList() throws IOException {
+		return readList(1);
+	}
+
+	private BdfList readList(int level) throws IOException {
 		if (!hasList()) throw new FormatException();
+		if (level > nestedLimit) throw new FormatException();
 		BdfList list = new BdfList();
 		readListStart();
-		while (!hasListEnd()) list.add(readObject());
+		while (!hasListEnd()) list.add(readObject(level + 1));
 		readListEnd();
 		return list;
 	}
@@ -333,11 +349,16 @@ class BdfReaderImpl implements BdfReader {
 	}
 
 	public BdfDictionary readDictionary() throws IOException {
+		return readDictionary(1);
+	}
+
+	private BdfDictionary readDictionary(int level) throws IOException {
 		if (!hasDictionary()) throw new FormatException();
+		if (level > nestedLimit) throw new FormatException();
 		BdfDictionary dictionary = new BdfDictionary();
 		readDictionaryStart();
 		while (!hasDictionaryEnd())
-			dictionary.put(readString(Integer.MAX_VALUE), readObject());
+			dictionary.put(readString(Integer.MAX_VALUE), readObject(level + 1));
 		readDictionaryEnd();
 		return dictionary;
 	}

briar-tests/src/org/briarproject/data/BdfReaderImplTest.java

@@ -11,6 +11,7 @@ import org.junit.Test;
 import java.io.ByteArrayInputStream;
 
 import static org.briarproject.api.data.BdfDictionary.NULL_VALUE;
+import static org.briarproject.data.BdfReaderImpl.DEFAULT_NESTED_LIMIT;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -472,9 +473,73 @@ public class BdfReaderImplTest extends BriarTestCase {
 		assertTrue(r.eof());
 	}
 
+	@Test
+	public void testNestedListWithinDepthLimit() throws Exception {
+		// A list containing a list containing a list containing a list...
+		String lists = "";
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT; i++) lists += "60";
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT; i++) lists += "80";
+		setContents(lists);
+		r.readList();
+		assertTrue(r.eof());
+	}
+
+	@Test(expected = FormatException.class)
+	public void testNestedListOutsideDepthLimit() throws Exception {
+		// A list containing a list containing a list containing a list...
+		String lists = "";
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT + 1; i++) lists += "60";
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT + 1; i++) lists += "80";
+		setContents(lists);
+		r.readList();
+	}
+
+	@Test
+	public void testNestedDictionaryWithinDepthLimit() throws Exception {
+		// A dictionary containing a dictionary containing a dictionary...
+		String dicts = "";
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT; i++)
+			dicts += "70" + "41" + "03" + "666F6F";
+		dicts += "11";
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT; i++)
+			dicts += "80";
+		setContents(dicts);
+		r.readDictionary();
+		assertTrue(r.eof());
+	}
+
+	@Test(expected = FormatException.class)
+	public void testNestedDictionaryOutsideDepthLimit() throws Exception {
+		// A dictionary containing a dictionary containing a dictionary...
+		String dicts = "";
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT + 1; i++)
+			dicts += "70" + "41" + "03" + "666F6F";
+		dicts += "11";
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT + 1; i++)
+			dicts += "80";
+		setContents(dicts);
+		r.readDictionary();
+	}
+
+	@Test(expected = FormatException.class)
+	public void testOpenList() throws Exception {
+		// A list that is not closed
+		String list = "60";
+		setContents(list);
+		r.readList();
+	}
+
+	@Test(expected = FormatException.class)
+	public void testOpenDictionary() throws Exception {
+		// A dictionary that is not closed
+		String dicts = "70" + "41" + "03" + "666F6F";
+		setContents(dicts);
+		r.readDictionary();
+	}
+
 	private void setContents(String hex) {
 		ByteArrayInputStream in = new ByteArrayInputStream(
 				StringUtils.fromHexString(hex));
-		r = new BdfReaderImpl(in);
+		r = new BdfReaderImpl(in, DEFAULT_NESTED_LIMIT);
 	}
 }