java.lang.OutOfMemoryError: Java heap space when decoding non GPP string

[alex-ro]

Add the following test in GppModelTest.java file.

  // java.lang.OutOfMemoryError: Java heap space
  @Test
  public void testDecodingExceptionValidStringButNotGPP() {
    try {
      GppModel gppModel = new GppModel("CP48G0AP48G0AEsACCPLAkEgAAAAAEPgAB5YAAAQaQD2F2K2kKFkPCmQWYAQBCijYEAhQAAAAkCBIAAgAUgQAgFIIAgAIFAAAAAAAAAQEgCQAAQABAAAIACgAAAAAAIAAAAAAAQQAAAAAIAAAAAAAAEAAAAAAAQAAAAIAABEhCAAQQAEAAAAAAAQAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAgAA");
      Assertions.fail("Expected LazyDecodingException");
    } catch (DecodingException e) {

    }
  }

The proposed solution is to validate the first 2 numbers in the header of the message, to fail fast if the string is not GPP.
So, in HeaderV1.java:

  private static final String ID_BITSTRING = "000011";
  private static final String VERSION_BITSTRING = "000001";
  private static final String HEADER_PREFIX_BITSTRING = ID_BITSTRING + VERSION_BITSTRING;
  @Override
  public void decode(String encodedString) throws DecodingException {
    String bitString = base64UrlEncoder.decode(encodedString);
    if (!bitString.startsWith(HEADER_PREFIX_BITSTRING)) {
      throw new DecodingException("Not GPP v1 string");
    };
    this.decodeFromBitString(bitString);
  }

[hhakimi53]

@chuff / @iabmayank : can someone please have a look at this as this may be potentially happening where the encoded GPP string passed can be without headers or malformed. Doing this integrity check within the library is better and more maintainable with upgrades to library.

[krjackso]

We are also seeing this issue

[ChristopherWirt]

We are also seeing this issue

[abazylewicz]

We are also seeing this issue. The proposed solution will help with obviously wrong GPP string, but one could still prepare a GPP string which starts with proper ID and version, to then create a range of millions of sections and crash the VM, so a bad actor could use this to easily crash the application.

[yuzawa-san]

I agree that further validation is necessary since we do not want a input string to cause infinite or deep regress, but the last version i was able to verify this was 3.1.1. Version 3.2.0 seems to be ok. Could you please verify? I only used the code snippet above to test, so maybe some other method (like a specific getter) triggers the OOME. I have a theory that maybe the lazy evaluation maybe fixed some aspect of this #34

[abazylewicz-id5]

Yes, after adding the lazy decoding no decoding takes place until some method is called to read the data. But eg. reading gppModel.getHeader().getSectionsIds() causes it to be decoded and still can throw an OOM.
There was some safeguard added in 3e70a3e#diff-52905c5e26d5015550baf7408f06f997d7b6b6c0a65b9bd36e44a00fdd0a7448R282-R283 - but it's enough to start the invalid gpp string with D to get the OOM again

[yuzawa-san]

I believe I have a fix for the OOME in #71 could you please review?

[yuzawa-san]

could you please take a look at #71 specifically the max size for ranges of 8192 which is roughly double the MSPA/GPP vendor list. if you have any concerns about that limit please discuss.