Sanity check on buffer allocation based for definite length sequences

[kimholan]

I am trying out jASN1 to convert non-streaming DER-encoded data to Java classes generated using an ASN.1-schema.

The current API has no features to sanity check the buffer allocation which can lead to memory exhaustion due to eagerly overallocating byte buffers.

Are you interested in modifying the API to specifify an upper limit to sanity check these kinds of length?

Example:

import com.beanit.jasn1.ber.types.BerOctetString;
import org.junit.Assert;
import org.junit.Test;

import java.io.ByteArrayInputStream;
import java.io.EOFException;
import java.io.IOException;

public class BerOctetStringTest {

    @Test
    public void test() throws IOException {
        BerOctetString reverseMalformed = new BerOctetString();
        try {
            byte[] malformed = {4, -124, 8, 0, 0, 0};
            reverseMalformed.decode(new ByteArrayInputStream(malformed));
            Assert.fail();
        } catch (EOFException cause) {
            // expected
        } finally {
            // buffer was already allocated
            Assert.assertEquals(134217728, reverseMalformed.value.length);
        }
    }
}`

[sfeuerhahn]

sounds like a good idea, but what should be the upper limit allowed? I don't want to limit the allowed use cases for the library.

[kimholan]

The simplest way would be to use a ByteArrayOutputStream, but it might be overdoing for when the byte arrays are small - and a lot of methods in BAOS are synchronized (although current JVMs should be optimize this).

The code gets a bit ugly though when wanting to offer several code paths. For example:

    if (length.val != 0) {
      if (length.val >  65535) {
        ByteArrayOutputStream bufferStream = new ByteArrayOutputStream();
        byte[] buffer= new byte[8192];
        int n;
        while ( (n=is.read(buffer, 0, buffer.length))!=-1 ) {
           bufferStream.write(buffer,0,n);
        }
        value = bufferStream.toByteArray();
      } else {
        Util.readFully(is,value);
      }

      codeLength += length.val;
    }

Maybe you should just ignore this. One can always just fork this and modify it for their own demands. :D

The code generator easy to understand as it is. :smile:

[sfeuerhahn]

I will leave that as it is for now. But keep it in mind for future revisions.