fix: nullptr checks and inputstream

[thanhdanh]

Summary

Fix all security comments at https://github.com/ExodusMovement/exodus-mobile/pull/23341#issuecomment-2595238966 and https://github.com/ExodusMovement/exodus-mobile/pull/23341#issuecomment-2602512158

• Add nullptr checks to all the memcpy operations
• Ensure the length does not exceed maximum buffer size.
• Use the totalBytesRead variable to keep a running total of the bytes read from the InputStream.
  • Read the input stream in chunks of 8192 bytes to avoid loading the entire input into memory at once.
  • After each read, add the bytesRead to totalBytesRead.
  • Check if totalBytesRead exceeds 100 MB. If it does, throw an exception to stop further processing.

[633kh4ck]

Better we handle the buffer size dynamically without using the hard-coded constant:

size_t length = 0;
const char *m_body = create_blocks_request(height, &length);

char *data = (char *) env->GetDirectBufferAddress(buf);
if (data == nullptr) {
    // Direct buffer address is not available
    return 0;
}

jlong capacity = env->GetDirectBufferCapacity(buf);
if (capacity < 0) {
    // If 'buf' is not a direct buffer, capacity could be -1;
    // Not expected here since data != nullptr, but just in case
    return 0;
}

// Safely clamp 'capacity' to SIZE_MAX to avoid overflow in case jlong > size_t
size_t max_length = (capacity > (jlong) SIZE_MAX)
    ? SIZE_MAX
    : (size_t) capacity;

if (length > max_length) {
    // Body size is bigger than the buffer capacity
    return 0;
}

memcpy(data, m_body, length);

return length;

Note

The above code is untested. Verify it is syntactically correct, i.e., it compiles and runs, before using it.

[thanhdanh]

Thanks, I updated the code. I added some error codes in C++, and throwing exceptions in Java ensures that the native code remains simple while giving the Java layer complete control over error handling and exception management

[thanhdanh]

Tested on Android with version 18.3.5-rc.1

[ale-exo]

Hey :) Would it be better to return when length > MAX_BUFFER_SIZE instead of continuing? Could this lead to copy corrupted/truncated data? In Android we handle this case by throwing an error (here and here). I haven't deeply checked create_blocks_request and GetDirectBufferAddress functions, but I think if the m_body has length > MAX_BUFFER_SIZE bytes, we'll copy MAX_BUFFER_SIZE bytes, meaning in this case length - MAX_BUFFER_SIZE bytes will not be copied.

[633kh4ck]

Good call out, I adjusted the above code snippet to address this concern 👍🏻