LUCENE-3738: DataInput/DataOutput no longer allow negative vLongs. vInts are still supported (for index backwards compatibility), but should not be used in new code. The read method for negative vLongs was already broken since Lucene 3.1

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1302238 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Uwe Schindler 2012-03-18 22:27:22 +00:00
parent 059d4bd1c3
commit 39e727069b
6 changed files with 163 additions and 46 deletions

View File

@ -796,6 +796,12 @@ Changes in Runtime Behavior
still check for and silently correct this situation today, but at some point
in the future they may throw an exception. (Mike McCandless, Robert Muir)
* LUCENE-3738: DataInput/DataOutput no longer allow negative vLongs.
vInts are still supported (for index backwards compatibility), but
should not be used in new code. The read method for negative vLongs
was already broken since Lucene 3.1.
(Uwe Schindler, Mike McCandless, Robert Muir)
Security fixes
* LUCENE-3588: Try harder to prevent SIGSEGV on cloned MMapIndexInputs:

View File

@ -46,7 +46,7 @@ public abstract class BufferedIndexInput extends IndexInput {
private int bufferPosition = 0; // next byte to read
@Override
public byte readByte() throws IOException {
public final byte readByte() throws IOException {
if (bufferPosition >= bufferLength)
refill();
return buffer[bufferPosition++];
@ -68,7 +68,7 @@ public abstract class BufferedIndexInput extends IndexInput {
}
/** Change the buffer size used by this IndexInput */
public void setBufferSize(int newSize) {
public final void setBufferSize(int newSize) {
assert buffer == null || bufferSize == buffer.length: "buffer=" + buffer + " bufferSize=" + bufferSize + " buffer.length=" + (buffer != null ? buffer.length : 0);
if (newSize != bufferSize) {
checkBufferSize(newSize);
@ -99,7 +99,7 @@ public abstract class BufferedIndexInput extends IndexInput {
}
/** Returns buffer size. @see #setBufferSize */
public int getBufferSize() {
public final int getBufferSize() {
return bufferSize;
}
@ -109,12 +109,12 @@ public abstract class BufferedIndexInput extends IndexInput {
}
@Override
public void readBytes(byte[] b, int offset, int len) throws IOException {
public final void readBytes(byte[] b, int offset, int len) throws IOException {
readBytes(b, offset, len, true);
}
@Override
public void readBytes(byte[] b, int offset, int len, boolean useBuffer) throws IOException {
public final void readBytes(byte[] b, int offset, int len, boolean useBuffer) throws IOException {
if(len <= (bufferLength-bufferPosition)){
// the buffer contains enough data to satisfy this request
@ -164,7 +164,7 @@ public abstract class BufferedIndexInput extends IndexInput {
}
@Override
public short readShort() throws IOException {
public final short readShort() throws IOException {
if (2 <= (bufferLength-bufferPosition)) {
return (short) (((buffer[bufferPosition++] & 0xFF) << 8) | (buffer[bufferPosition++] & 0xFF));
} else {
@ -173,7 +173,7 @@ public abstract class BufferedIndexInput extends IndexInput {
}
@Override
public int readInt() throws IOException {
public final int readInt() throws IOException {
if (4 <= (bufferLength-bufferPosition)) {
return ((buffer[bufferPosition++] & 0xFF) << 24) | ((buffer[bufferPosition++] & 0xFF) << 16)
| ((buffer[bufferPosition++] & 0xFF) << 8) | (buffer[bufferPosition++] & 0xFF);
@ -183,7 +183,7 @@ public abstract class BufferedIndexInput extends IndexInput {
}
@Override
public long readLong() throws IOException {
public final long readLong() throws IOException {
if (8 <= (bufferLength-bufferPosition)) {
final int i1 = ((buffer[bufferPosition++] & 0xff) << 24) | ((buffer[bufferPosition++] & 0xff) << 16) |
((buffer[bufferPosition++] & 0xff) << 8) | (buffer[bufferPosition++] & 0xff);
@ -196,30 +196,61 @@ public abstract class BufferedIndexInput extends IndexInput {
}
@Override
public int readVInt() throws IOException {
public final int readVInt() throws IOException {
if (5 <= (bufferLength-bufferPosition)) {
byte b = buffer[bufferPosition++];
int i = b & 0x7F;
for (int shift = 7; (b & 0x80) != 0; shift += 7) {
b = buffer[bufferPosition++];
i |= (b & 0x7F) << shift;
}
return i;
if ((b & 0x80) == 0) return i;
b = buffer[bufferPosition++];
i |= (b & 0x7F) << 7;
if ((b & 0x80) == 0) return i;
b = buffer[bufferPosition++];
i |= (b & 0x7F) << 14;
if ((b & 0x80) == 0) return i;
b = buffer[bufferPosition++];
i |= (b & 0x7F) << 21;
if ((b & 0x80) == 0) return i;
b = buffer[bufferPosition++];
// Warning: the next ands use 0x0F / 0xF0 - beware copy/paste errors:
i |= (b & 0x0F) << 28;
if ((b & 0xF0) == 0) return i;
throw new IOException("Invalid vInt detected (too many bits)");
} else {
return super.readVInt();
}
}
@Override
public long readVLong() throws IOException {
public final long readVLong() throws IOException {
if (9 <= bufferLength-bufferPosition) {
byte b = buffer[bufferPosition++];
long i = b & 0x7F;
for (int shift = 7; (b & 0x80) != 0; shift += 7) {
b = buffer[bufferPosition++];
i |= (b & 0x7FL) << shift;
}
return i;
long i = b & 0x7FL;
if ((b & 0x80) == 0) return i;
b = buffer[bufferPosition++];
i |= (b & 0x7FL) << 7;
if ((b & 0x80) == 0) return i;
b = buffer[bufferPosition++];
i |= (b & 0x7FL) << 14;
if ((b & 0x80) == 0) return i;
b = buffer[bufferPosition++];
i |= (b & 0x7FL) << 21;
if ((b & 0x80) == 0) return i;
b = buffer[bufferPosition++];
i |= (b & 0x7FL) << 28;
if ((b & 0x80) == 0) return i;
b = buffer[bufferPosition++];
i |= (b & 0x7FL) << 35;
if ((b & 0x80) == 0) return i;
b = buffer[bufferPosition++];
i |= (b & 0x7FL) << 42;
if ((b & 0x80) == 0) return i;
b = buffer[bufferPosition++];
i |= (b & 0x7FL) << 49;
if ((b & 0x80) == 0) return i;
b = buffer[bufferPosition++];
i |= (b & 0x7FL) << 56;
if ((b & 0x80) == 0) return i;
throw new IOException("Invalid vLong detected (negative values disallowed)");
} else {
return super.readVLong();
}
@ -254,10 +285,10 @@ public abstract class BufferedIndexInput extends IndexInput {
throws IOException;
@Override
public long getFilePointer() { return bufferStart + bufferPosition; }
public final long getFilePointer() { return bufferStart + bufferPosition; }
@Override
public void seek(long pos) throws IOException {
public final void seek(long pos) throws IOException {
if (pos >= bufferStart && pos < (bufferStart + bufferLength))
bufferPosition = (int)(pos - bufferStart); // seek within buffer
else {
@ -295,7 +326,7 @@ public abstract class BufferedIndexInput extends IndexInput {
*
* @return the number of bytes actually flushed from the in-memory buffer.
*/
protected int flushBuffer(IndexOutput out, long numBytes) throws IOException {
protected final int flushBuffer(IndexOutput out, long numBytes) throws IOException {
int toCopy = bufferLength - bufferPosition;
if (toCopy > numBytes) {
toCopy = (int) numBytes;

View File

@ -17,6 +17,8 @@ package org.apache.lucene.store;
* limitations under the License.
*/
import java.io.IOException;
import org.apache.lucene.util.BytesRef;
/** @lucene.experimental */
@ -103,25 +105,66 @@ public final class ByteArrayDataInput extends DataInput {
assert checkBounds();
byte b = bytes[pos++];
int i = b & 0x7F;
for (int shift = 7; (b & 0x80) != 0; shift += 7) {
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7F) << shift;
}
return i;
if ((b & 0x80) == 0) return i;
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7F) << 7;
if ((b & 0x80) == 0) return i;
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7F) << 14;
if ((b & 0x80) == 0) return i;
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7F) << 21;
if ((b & 0x80) == 0) return i;
assert checkBounds();
b = bytes[pos++];
// Warning: the next ands use 0x0F / 0xF0 - beware copy/paste errors:
i |= (b & 0x0F) << 28;
if ((b & 0xF0) == 0) return i;
throw new RuntimeException("Invalid vInt detected (too many bits)");
}
@Override
public long readVLong() {
assert checkBounds();
byte b = bytes[pos++];
long i = b & 0x7F;
for (int shift = 7; (b & 0x80) != 0; shift += 7) {
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7FL) << shift;
}
return i;
long i = b & 0x7FL;
if ((b & 0x80) == 0) return i;
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7FL) << 7;
if ((b & 0x80) == 0) return i;
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7FL) << 14;
if ((b & 0x80) == 0) return i;
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7FL) << 21;
if ((b & 0x80) == 0) return i;
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7FL) << 28;
if ((b & 0x80) == 0) return i;
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7FL) << 35;
if ((b & 0x80) == 0) return i;
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7FL) << 42;
if ((b & 0x80) == 0) return i;
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7FL) << 49;
if ((b & 0x80) == 0) return i;
assert checkBounds();
b = bytes[pos++];
i |= (b & 0x7FL) << 56;
if ((b & 0x80) == 0) return i;
throw new RuntimeException("Invalid vLong detected (negative values disallowed)");
}
// NOTE: AIOOBE not EOF if you read too much

View File

@ -106,8 +106,10 @@ public abstract class DataInput implements Cloneable {
i |= (b & 0x7F) << 21;
if ((b & 0x80) == 0) return i;
b = readByte();
assert (b & 0x80) == 0;
return i | ((b & 0x7F) << 28);
// Warning: the next ands use 0x0F / 0xF0 - beware copy/paste errors:
i |= (b & 0x0F) << 28;
if ((b & 0xF0) == 0) return i;
throw new IOException("Invalid vInt detected (too many bits)");
}
/** Reads eight bytes and returns a long.
@ -157,8 +159,9 @@ public abstract class DataInput implements Cloneable {
i |= (b & 0x7FL) << 49;
if ((b & 0x80) == 0) return i;
b = readByte();
assert (b & 0x80) == 0;
return i | ((b & 0x7FL) << 56);
i |= (b & 0x7FL) << 56;
if ((b & 0x80) == 0) return i;
throw new IOException("Invalid vLong detected (negative values disallowed)");
}
/** Reads a string.

View File

@ -70,8 +70,8 @@ public abstract class DataOutput {
}
/** Writes an int in a variable-length format. Writes between one and
* five bytes. Smaller values take fewer bytes. Negative numbers are not
* supported.
* five bytes. Smaller values take fewer bytes. Negative numbers are
* supported, but should be avoided.
* @see DataInput#readVInt()
*/
public final void writeVInt(int i) throws IOException {
@ -96,6 +96,7 @@ public abstract class DataOutput {
* @see DataInput#readVLong()
*/
public final void writeVLong(long i) throws IOException {
assert i >= 0L;
while ((i & ~0x7F) != 0) {
writeByte((byte)((i & 0x7f) | 0x80));
i >>>= 7;

View File

@ -18,6 +18,8 @@ package org.apache.lucene.index;
*/
import org.apache.lucene.util.LuceneTestCase;
import org.apache.lucene.store.ByteArrayDataInput;
import org.apache.lucene.store.DataInput;
import org.apache.lucene.store.IndexInput;
import org.apache.lucene.store.IndexOutput;
import org.apache.lucene.store.RAMDirectory;
@ -32,6 +34,7 @@ public class TestIndexInput extends LuceneTestCase {
(byte) 0x80, (byte) 0x80, 0x01,
(byte) 0x81, (byte) 0x80, 0x01,
(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0x07,
(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0x0F,
(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0x07,
(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0x7F,
0x06, 'L', 'u', 'c', 'e', 'n', 'e',
@ -63,14 +66,21 @@ public class TestIndexInput extends LuceneTestCase {
// null bytes
0x01, 0x00,
0x08, 'L', 'u', 0x00, 'c', 'e', 0x00, 'n', 'e',
// tests for Exceptions on invalid values
(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0x17,
(byte) 0x01, // guard value
(byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF,
(byte) 0x01, // guard value
};
private void checkReads(IndexInput is) throws IOException {
private void checkReads(DataInput is, Class<? extends Exception> expectedEx) throws IOException {
assertEquals(128,is.readVInt());
assertEquals(16383,is.readVInt());
assertEquals(16384,is.readVInt());
assertEquals(16385,is.readVInt());
assertEquals(Integer.MAX_VALUE, is.readVInt());
assertEquals(-1, is.readVInt());
assertEquals((long) Integer.MAX_VALUE, is.readVLong());
assertEquals(Long.MAX_VALUE, is.readVLong());
assertEquals("Lucene",is.readString());
@ -87,12 +97,30 @@ public class TestIndexInput extends LuceneTestCase {
assertEquals("\u0000",is.readString());
assertEquals("Lu\u0000ce\u0000ne",is.readString());
try {
is.readVInt();
fail("Should throw " + expectedEx.getName());
} catch (Exception e) {
assertTrue(e.getMessage().startsWith("Invalid vInt"));
assertTrue(expectedEx.isInstance(e));
}
assertEquals(1, is.readVInt()); // guard value
try {
is.readVLong();
fail("Should throw " + expectedEx.getName());
} catch (Exception e) {
assertTrue(e.getMessage().startsWith("Invalid vLong"));
assertTrue(expectedEx.isInstance(e));
}
assertEquals(1L, is.readVLong()); // guard value
}
// this test only checks BufferedIndexInput because MockIndexInput extends BufferedIndexInput
public void testBufferedIndexInputRead() throws IOException {
final IndexInput is = new MockIndexInput(READ_TEST_BYTES);
checkReads(is);
checkReads(is, IOException.class);
is.close();
}
@ -103,9 +131,14 @@ public class TestIndexInput extends LuceneTestCase {
os.writeBytes(READ_TEST_BYTES, READ_TEST_BYTES.length);
os.close();
final IndexInput is = dir.openInput("foo", newIOContext(random));
checkReads(is);
checkReads(is, IOException.class);
is.close();
dir.close();
}
public void testByteArrayDataInput() throws IOException {
final ByteArrayDataInput is = new ByteArrayDataInput(READ_TEST_BYTES);
checkReads(is, RuntimeException.class);
}
}