You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Hao Zhong (JIRA)" <ji...@apache.org> on 2018/10/18 08:48:00 UTC

[jira] [Updated] (LUCENE-8536) The bytes parameter of the copy method is not carefully checked.

     [ https://issues.apache.org/jira/browse/LUCENE-8536?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hao Zhong updated LUCENE-8536:
------------------------------
    Summary: The bytes parameter of the copy method is not carefully checked.  (was: The bytes parameter of the copy can be overflowed.)

> The bytes parameter of the copy method is not carefully checked.
> ----------------------------------------------------------------
>
>                 Key: LUCENE-8536
>                 URL: https://issues.apache.org/jira/browse/LUCENE-8536
>             Project: Lucene - Core
>          Issue Type: Bug
>          Components: -tools
>    Affects Versions: trunk
>            Reporter: Hao Zhong
>            Priority: Major
>
> The copy method of the PagedBytes class is as follow:
>  
>  
> {code:java}
> public void copy(BytesRef bytes, BytesRef out) {
> int left = blockSize - upto;
> if (bytes.length > left || currentBlock==null) {
> if (currentBlock != null) {
> addBlock(currentBlock);
> didSkipBytes = true;
> }
> currentBlock = new byte[blockSize];
> upto = 0;
> left = blockSize;
> assert bytes.length <= blockSize;
> // TODO: we could also support variable block sizes
> }
> out.bytes = currentBlock;
> out.offset = upto;
> out.length = bytes.length;
> System.arraycopy(bytes.bytes, bytes.offset, currentBlock, upto, bytes.length);
> upto += bytes.length;
> }
> {code}
> The method does not throw exceptions for illegal inputs. In the same class, the copyUsingLengthPrefix method checks the input value"
>  
>  
>  
> {code:java}
> public long copyUsingLengthPrefix(BytesRef bytes) {
> if (bytes.length >= 32768) {
> throw new IllegalArgumentException("max length is 32767 (got " + bytes.length + ")");
> }
> if (upto + bytes.length + 2 > blockSize) {
> if (bytes.length + 2 > blockSize) {
> throw new IllegalArgumentException("block size " + blockSize + " is too small to store length " + bytes.length + " bytes");
> }
> if (currentBlock != null) {
> addBlock(currentBlock); 
> }
> currentBlock = new byte[blockSize];
> upto = 0;
> }
> final long pointer = getPointer();
> if (bytes.length < 128) {
> currentBlock[upto++] = (byte) bytes.length;
> } else {
> currentBlock[upto++] = (byte) (0x80 | (bytes.length >> 8));
> currentBlock[upto++] = (byte) (bytes.length & 0xff);
> }
> System.arraycopy(bytes.bytes, bytes.offset, currentBlock, upto, bytes.length);
> upto += bytes.length;
> return pointer;
> }
> {code}
> I understand that in the first method, 
> {code:java}
> assert bytes.length <= blockSize;{code}
> checks whether the length of the bytes is too large. However,  the method does not check blockSize either. As a result, the length of the bytes can still be overflowed, if blockSize is too large.  In addition, the second method also checks whether 
> {code:java}
> bytes.length + 2 > blockSize{code}
> Shall the first method also checks the requirement?
>  
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org