Skip to content

Commit

Permalink
Fix unsafe cast and detect resize overflow. (#31106)
Browse files Browse the repository at this point in the history
Summary:
Removing unsafe cast from `int` to `uint16_t`.
Also, adding code to detect multiplication overflow during buffer resize.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General] [Fix] - Fix unsafe cast and detect overflow in MapBuffer.

Pull Request resolved: #31106

Test Plan: Code compiles in Visual Studio 2019 without the unsafe cast warning (or error depending on the configuration).

Reviewed By: mdvacca

Differential Revision: D26865138

Pulled By: rozele

fbshipit-source-id: 4692a38b05fc873e31fbbe94d70803244e82de5d
  • Loading branch information
Igor Klemenski authored and facebook-github-bot committed Mar 9, 2021
1 parent d4d2e90 commit e69f1c9
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 3 deletions.
9 changes: 8 additions & 1 deletion ReactCommon/react/renderer/mapbuffer/MapBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,21 @@ using namespace facebook::react;
namespace facebook {
namespace react {

MapBuffer::MapBuffer(int initialSize) {
MapBuffer::MapBuffer(uint16_t initialSize) {
_dataSize = initialSize;
_data = new Byte[_dataSize];
// TODO: Should we clean up memory here?
}

void MapBuffer::makeSpace() {
int oldDataSize = _dataSize;
if (_dataSize >= std::numeric_limits<uint16_t>::max() / 2) {
LOG(ERROR)
<< "Error: trying to assign a value beyond the capacity of uint16_t"
<< static_cast<uint32_t>(_dataSize) * 2;
throw "Error: trying to assign a value beyond the capacity of uint16_t" +
std::to_string(static_cast<uint32_t>(_dataSize) * 2);
}
_dataSize *= 2;
uint8_t *_newdata = new Byte[_dataSize];
uint8_t *_oldData = _data;
Expand Down
4 changes: 2 additions & 2 deletions ReactCommon/react/renderer/mapbuffer/MapBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace facebook {
namespace react {

// 506 = 5 entries = 50*10 + 6 sizeof(header)
const int INITIAL_SIZE = 506;
constexpr uint16_t INITIAL_SIZE = 506;

/**
* MapBuffer is an optimized map format for transferring data like props between
Expand Down Expand Up @@ -46,7 +46,7 @@ class MapBuffer {
public:
MapBuffer() : MapBuffer(INITIAL_SIZE) {}

MapBuffer(int initialSize);
MapBuffer(uint16_t initialSize);

~MapBuffer();

Expand Down

0 comments on commit e69f1c9

Please sign in to comment.