Issue 72: fix for infinite loop if allocate too large a buffer (due to invalid packet size)

This commit is contained in:
Aled Sage
2012-04-26 11:43:29 +01:00
parent ab04596a20
commit fb690c4fb0
4 changed files with 119 additions and 6 deletions

View File

@@ -74,10 +74,15 @@ public class Buffer<T extends Buffer<T>> {
/** The default size for a {@code Buffer} (256 bytes) */
public static final int DEFAULT_SIZE = 256;
/** The maximum valid size of buffer (i.e. biggest power of two that can be represented as an int - 2^30) */
public static final int MAX_SIZE = (1 << 30);
protected static int getNextPowerOf2(int i) {
int j = 1;
while (j < i)
while (j < i) {
j <<= 1;
if (j <= 0) throw new IllegalArgumentException("Cannot get next power of 2; "+i+" is too large");
}
return j;
}

View File

@@ -15,15 +15,16 @@
*/
package net.schmizz.sshj.sftp;
import net.schmizz.concurrent.Promise;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.io.InputStream;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import net.schmizz.concurrent.Promise;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class PacketReader
extends Thread {
@@ -65,7 +66,10 @@ public class PacketReader
public SFTPPacket<Response> readPacket()
throws IOException {
int len = getPacketLength();
if (len > SFTPPacket.MAX_SIZE) {
throw new IllegalStateException("Invalid packet: indicated length "+len+" too large");
}
packet.rpos(0);
packet.wpos(0);

View File

@@ -0,0 +1,33 @@
package net.schmizz.sshj.common;
import static org.junit.Assert.fail;
import net.schmizz.sshj.common.Buffer.PlainBuffer;
import org.junit.Test;
public class BufferTest {
// Issue 72: previously, it entered an infinite loop trying to establish the buffer size
@Test
public void shouldThrowOnTooLargeCapacity() {
PlainBuffer buffer = new PlainBuffer();
try {
buffer.ensureCapacity(Integer.MAX_VALUE);
fail("Allegedly ensured buffer capacity of size " + Integer.MAX_VALUE);
} catch (IllegalArgumentException e) {
// success
}
}
// Issue 72: previously, it entered an infinite loop trying to establish the buffer size
@Test
public void shouldThrowOnTooLargeInitialCapacity() {
try {
new PlainBuffer(Integer.MAX_VALUE);
fail("Allegedly created buffer with size " + Integer.MAX_VALUE);
} catch (IllegalArgumentException e) {
// success
}
}
}

View File

@@ -0,0 +1,71 @@
package net.schmizz.sshj.sftp;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.io.DataOutputStream;
import java.io.PipedInputStream;
import java.io.PipedOutputStream;
import java.util.Arrays;
import net.schmizz.sshj.connection.channel.direct.Session.Subsystem;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;
public class PacketReaderTest {
private DataOutputStream dataout;
private PacketReader reader;
private SFTPEngine engine;
private Subsystem subsystem;
@Before
public void setUp() throws Exception {
PipedOutputStream pipedout = new PipedOutputStream();
PipedInputStream pipedin = new PipedInputStream(pipedout);
dataout = new DataOutputStream(pipedout);
engine = Mockito.mock(SFTPEngine.class);
subsystem = Mockito.mock(Subsystem.class);
Mockito.when(engine.getSubsystem()).thenReturn(subsystem);
Mockito.when(subsystem.getInputStream()).thenReturn(pipedin);
reader = new PacketReader(engine);
}
// FIXME What is the byte format for the size? Big endian? Little endian?
@Test
public void shouldReadPacket() throws Exception {
byte[] bytes = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
dataout.writeInt(10);
dataout.write(bytes);
dataout.flush();
SFTPPacket<Response> packet = reader.readPacket();
assertEquals(packet.available(), 10);
assertTrue("actual=" + Arrays.toString(packet.array()), Arrays.equals(bytes, subArray(packet.array(), 0, 10)));
}
@Test
public void shouldFailWhenPacketLengthTooLarge() throws Exception {
dataout.writeInt(Integer.MAX_VALUE);
dataout.flush();
try {
reader.readPacket();
fail("Should have failed to read packet of size " + Integer.MAX_VALUE);
} catch (IllegalStateException e) {
e.printStackTrace();
// success; indicated packet size was too large
}
}
private byte[] subArray(byte[] source, int startIndex, int length) {
byte[] result = new byte[length];
System.arraycopy(source, startIndex, result, 0, length);
return result;
}
}