You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by us...@apache.org on 2012/03/18 23:27:23 UTC
svn commit: r1302238 - in /lucene/dev/trunk/lucene: ./
core/src/java/org/apache/lucene/store/ core/src/test/org/apache/lucene/index/
Author: uschindler
Date: Sun Mar 18 22:27:22 2012
New Revision: 1302238
URL: http://svn.apache.org/viewvc?rev=1302238&view=rev
Log:
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
Modified:
lucene/dev/trunk/lucene/CHANGES.txt
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/BufferedIndexInput.java
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/ByteArrayDataInput.java
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/DataInput.java
lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/DataOutput.java
lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexInput.java
Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1302238&r1=1302237&r2=1302238&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Sun Mar 18 22:27:22 2012
@@ -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:
Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/BufferedIndexInput.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/BufferedIndexInput.java?rev=1302238&r1=1302237&r2=1302238&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/BufferedIndexInput.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/BufferedIndexInput.java Sun Mar 18 22:27:22 2012
@@ -46,7 +46,7 @@ public abstract class BufferedIndexInput
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
}
/** 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
}
/** Returns buffer size. @see #setBufferSize */
- public int getBufferSize() {
+ public final int getBufferSize() {
return bufferSize;
}
@@ -109,12 +109,12 @@ public abstract class BufferedIndexInput
}
@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
}
@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
}
@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
}
@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
}
@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
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
*
* @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;
Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/ByteArrayDataInput.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/ByteArrayDataInput.java?rev=1302238&r1=1302237&r2=1302238&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/ByteArrayDataInput.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/ByteArrayDataInput.java Sun Mar 18 22:27:22 2012
@@ -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 ex
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
Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/DataInput.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/DataInput.java?rev=1302238&r1=1302237&r2=1302238&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/DataInput.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/DataInput.java Sun Mar 18 22:27:22 2012
@@ -106,8 +106,10 @@ public abstract class DataInput implemen
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 implemen
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.
Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/DataOutput.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/DataOutput.java?rev=1302238&r1=1302237&r2=1302238&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/DataOutput.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/DataOutput.java Sun Mar 18 22:27:22 2012
@@ -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;
Modified: lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexInput.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexInput.java?rev=1302238&r1=1302237&r2=1302238&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexInput.java (original)
+++ lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexInput.java Sun Mar 18 22:27:22 2012
@@ -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 Luce
(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 Luce
// 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 Luce
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 Luce
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);
+ }
+
}