Allow a maximum length to be specified when reading strings or byte

arrays, check it before allocating the buffer, and always specify the
maximum length when reading untrusted data - otherwise
CountingConsumer will reject the packet, but not before we've tried to
allocate a buffer of the specified size (up to 2 GB).
This commit is contained in:
akwizgran
2011-08-03 19:29:30 +01:00
parent 5fd87647f8
commit c90a18278b
13 changed files with 73 additions and 31 deletions

View File

@@ -2,11 +2,24 @@ package net.sf.briar.api.protocol;
import net.sf.briar.api.serial.Writable;
/** A pseudonymous author of messages. */
public interface Author extends Writable {
/** The maximum length of an author's name, in UTF-8 bytes. */
static final int MAX_NAME_LENGTH = 50;
/** The maximum length of an author's public key, in bytes. */
static final int MAX_PUBLIC_KEY_LENGTH = 1000;
/** Returns the author's unique identifier. */
AuthorId getId();
/** Returns the author's name. */
String getName();
/**
* Returns the public key that is used to verify messages signed by the
* author.
*/
byte[] getPublicKey();
}

View File

@@ -5,6 +5,12 @@ import net.sf.briar.api.serial.Writable;
/** A group to which users may subscribe. */
public interface Group extends Writable {
/** The maximum length of a group's name, in UTF-8 bytes. */
static final int MAX_NAME_LENGTH = 50;
/** The maximum length of a group's public key, in bytes. */
static final int MAX_PUBLIC_KEY_LENGTH = 1000;
/** Returns the group's unique identifier. */
GroupId getId();

View File

@@ -2,8 +2,12 @@ package net.sf.briar.api.protocol;
public interface Message {
/** The maximum size of a serialised message, in bytes. */
static final int MAX_SIZE = (1024 * 1024) - 200;
/** The maximum size of a signature, in bytes. */
static final int MAX_SIGNATURE_SIZE = 100;
/** Returns the message's unique identifier. */
MessageId getId();

View File

@@ -38,8 +38,11 @@ public interface Reader {
boolean hasString() throws IOException;
String readString() throws IOException;
String readString(int maxLength) throws IOException;
boolean hasBytes() throws IOException;
byte[] readBytes() throws IOException;
byte[] readBytes(int maxLength) throws IOException;
boolean hasList() throws IOException;
List<Object> readList() throws IOException;

View File

@@ -31,8 +31,8 @@ class AuthorReader implements ObjectReader<Author> {
// Read and digest the data
r.addConsumer(digesting);
r.readUserDefinedTag(Tags.AUTHOR);
String name = r.readString();
byte[] publicKey = r.readBytes();
String name = r.readString(Author.MAX_NAME_LENGTH);
byte[] publicKey = r.readBytes(Author.MAX_PUBLIC_KEY_LENGTH);
r.removeConsumer(digesting);
// Build and return the author
AuthorId id = new AuthorId(messageDigest.digest());

View File

@@ -13,7 +13,7 @@ class BatchIdReader implements ObjectReader<BatchId> {
public BatchId readObject(Reader r) throws IOException {
r.readUserDefinedTag(Tags.BATCH_ID);
byte[] b = r.readBytes();
byte[] b = r.readBytes(UniqueId.LENGTH);
if(b.length != UniqueId.LENGTH) throw new FormatException();
return new BatchId(b);
}

View File

@@ -13,7 +13,7 @@ class GroupIdReader implements ObjectReader<GroupId> {
public GroupId readObject(Reader r) throws IOException {
r.readUserDefinedTag(Tags.GROUP_ID);
byte[] b = r.readBytes();
byte[] b = r.readBytes(UniqueId.LENGTH);
if(b.length != UniqueId.LENGTH) throw new FormatException();
return new GroupId(b);
}

View File

@@ -31,10 +31,10 @@ class GroupReader implements ObjectReader<Group> {
// Read and digest the data
r.addConsumer(digesting);
r.readUserDefinedTag(Tags.GROUP);
String name = r.readString();
String name = r.readString(Group.MAX_NAME_LENGTH);
byte[] publicKey = null;
if(r.hasNull()) r.readNull();
else publicKey = r.readBytes();
else publicKey = r.readBytes(Group.MAX_PUBLIC_KEY_LENGTH);
r.removeConsumer(digesting);
// Build and return the group
GroupId id = new GroupId(messageDigest.digest());

View File

@@ -13,7 +13,7 @@ class MessageIdReader implements ObjectReader<MessageId> {
public MessageId readObject(Reader r) throws IOException {
r.readUserDefinedTag(Tags.MESSAGE_ID);
byte[] b = r.readBytes();
byte[] b = r.readBytes(UniqueId.LENGTH);
if(b.length != UniqueId.LENGTH) throw new FormatException();
return new MessageId(b);
}

View File

@@ -67,19 +67,19 @@ class MessageReader implements ObjectReader<Message> {
long timestamp = r.readInt64();
if(timestamp < 0L) throw new FormatException();
// Skip the message body
r.readBytes();
r.readBytes(Message.MAX_SIZE);
// Record the length of the data covered by the author's signature
int signedByAuthor = (int) counting.getCount();
// Read the author's signature, if there is one
byte[] authorSig = null;
if(author == null) r.readNull();
else authorSig = r.readBytes();
else authorSig = r.readBytes(Message.MAX_SIGNATURE_SIZE);
// Record the length of the data covered by the group's signature
int signedByGroup = (int) counting.getCount();
// Read the group's signature, if there is one
byte[] groupSig = null;
if(group.getPublicKey() == null) r.readNull();
else groupSig = r.readBytes();
else groupSig = r.readBytes(Message.MAX_SIGNATURE_SIZE);
// That's all, folks
r.removeConsumer(counting);
r.removeConsumer(copying);

View File

@@ -26,7 +26,7 @@ class RequestReader implements ObjectReader<Request> {
// Read the data
r.addConsumer(counting);
r.readUserDefinedTag(Tags.REQUEST);
byte[] bitmap = r.readBytes();
byte[] bitmap = r.readBytes(Request.MAX_SIZE);
r.removeConsumer(counting);
// Convert the bitmap into a BitSet
BitSet b = new BitSet(bitmap.length * 8);

View File

@@ -268,18 +268,16 @@ class ReaderImpl implements Reader {
}
public String readString() throws IOException {
if(!hasString()) throw new FormatException();
consumeLookahead();
if(next == Tag.STRING) {
return readString(readLength());
} else {
int length = 0xFF & next ^ Tag.SHORT_STRING;
return readString(length);
}
return readString(Integer.MAX_VALUE);
}
private String readString(int length) throws IOException {
assert length >= 0;
public String readString(int maxLength) throws IOException {
if(!hasString()) throw new FormatException();
consumeLookahead();
int length;
if(next == Tag.STRING) length = readLength();
else length = 0xFF & next ^ Tag.SHORT_STRING;
if(length > maxLength) throw new FormatException();
if(length == 0) return "";
readIntoBuffer(length);
return new String(buf, 0, length, "UTF-8");
@@ -308,18 +306,16 @@ class ReaderImpl implements Reader {
}
public byte[] readBytes() throws IOException {
if(!hasBytes()) throw new FormatException();
consumeLookahead();
if(next == Tag.BYTES) {
return readBytes(readLength());
} else {
int length = 0xFF & next ^ Tag.SHORT_BYTES;
return readBytes(length);
}
return readBytes(Integer.MAX_VALUE);
}
private byte[] readBytes(int length) throws IOException {
assert length >= 0;
public byte[] readBytes(int maxLength) throws IOException {
if(!hasBytes()) throw new FormatException();
consumeLookahead();
int length;
if(next == Tag.BYTES) length = readLength();
else length = 0xFF & next ^ Tag.SHORT_BYTES;
if(length > maxLength) throw new FormatException();
if(length == 0) return EMPTY_BUFFER;
byte[] b = new byte[length];
readIntoBuffer(b, length);

View File

@@ -132,6 +132,16 @@ public class ReaderImplTest extends TestCase {
assertTrue(r.eof());
}
@Test
public void testReadStringMaxLength() throws Exception {
setContents("83666F6F" + "83666F6F");
assertEquals("foo", r.readString(3));
try {
r.readString(2);
assertTrue(false);
} catch(FormatException expected) {}
}
@Test
public void testReadBytes() throws Exception {
setContents("F603010203" + "93010203" + "F600" + "90");
@@ -142,6 +152,16 @@ public class ReaderImplTest extends TestCase {
assertTrue(r.eof());
}
@Test
public void testReadBytesMaxLength() throws Exception {
setContents("93010203" + "93010203");
assertTrue(Arrays.equals(new byte[] {1, 2, 3}, r.readBytes(3)));
try {
r.readBytes(2);
assertTrue(false);
} catch(FormatException expected) {}
}
@Test
public void testReadShortList() throws Exception {
setContents("A" + "3" + "01" + "83666F6F" + "FC0080");