Skip to content

Commit 68a60ba

Browse files
author
Nitin Chaudhary
committed
Address remaining 5 PR review comments
- WinRTHttpResource: Validate requestId BEFORE casting to prevent overflow bypass - ImageViewManagerModule: Add MAX_DATA_URI_SIZE validation to prevent DoS (4 functions) - LinkingManagerModule: Use centralized AllowedSchemes::LINKING_SCHEMES constant - WebSocketJSExecutor: Add explanatory comment that file:// is debug-only - InputValidation.h: Add ms-settings to LINKING_SCHEMES for Windows deep linking Addresses PR feedback from @anupriya13 and Copilot AI review
1 parent 6f816da commit 68a60ba

File tree

5 files changed

+42
-20
lines changed

5 files changed

+42
-20
lines changed

‎vnext/Microsoft.ReactNative/Modules/ImageViewManagerModule.cpp‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,12 @@ void ImageLoader::Initialize(React::ReactContext const &reactContext) noexcept {
106106
void ImageLoader::getSize(std::string uri, React::ReactPromise<std::vector<double>> &&result) noexcept {
107107
// VALIDATE URI - file:// abuse PROTECTION (P0 Critical - CVSS 7.8)
108108
try {
109-
// Allow data: URIs and http/https only
110-
if (uri.find("data:") != 0) {
109+
if (uri.find("data:") == 0) {
110+
// Validate data URI size to prevent DoS through memory exhaustion
111+
::Microsoft::ReactNative::InputValidation::SizeValidator::ValidateSize(
112+
uri.length(), ::Microsoft::ReactNative::InputValidation::SizeValidator::MAX_DATA_URI_SIZE, "Data URI");
113+
} else {
114+
// Allow http/https only for non-data URIs
111115
::Microsoft::ReactNative::InputValidation::URLValidator::ValidateURL(uri, {"http", "https"});
112116
}
113117
} catch (const ::Microsoft::ReactNative::InputValidation::ValidationException &ex) {
@@ -140,8 +144,12 @@ void ImageLoader::getSizeWithHeaders(
140144
&&result) noexcept {
141145
// SDL Compliance: Validate URI for SSRF (P0 Critical - CVSS 7.8)
142146
try {
143-
// Allow data: URIs and http/https only
144-
if (uri.find("data:") != 0) {
147+
if (uri.find("data:") == 0) {
148+
// Validate data URI size to prevent DoS through memory exhaustion
149+
::Microsoft::ReactNative::InputValidation::SizeValidator::ValidateSize(
150+
uri.length(), ::Microsoft::ReactNative::InputValidation::SizeValidator::MAX_DATA_URI_SIZE, "Data URI");
151+
} else {
152+
// Allow http/https only for non-data URIs
145153
::Microsoft::ReactNative::InputValidation::URLValidator::ValidateURL(uri, {"http", "https"});
146154
}
147155
} catch (const ::Microsoft::ReactNative::InputValidation::ValidationException &ex) {
@@ -172,8 +180,12 @@ void ImageLoader::getSizeWithHeaders(
172180
void ImageLoader::prefetchImage(std::string uri, React::ReactPromise<bool> &&result) noexcept {
173181
// VALIDATE URI - file:// abuse PROTECTION (P0 Critical - CVSS 7.8)
174182
try {
175-
// Allow data: URIs and http/https only
176-
if (uri.find("data:") != 0) {
183+
if (uri.find("data:") == 0) {
184+
// Validate data URI size to prevent DoS through memory exhaustion
185+
::Microsoft::ReactNative::InputValidation::SizeValidator::ValidateSize(
186+
uri.length(), ::Microsoft::ReactNative::InputValidation::SizeValidator::MAX_DATA_URI_SIZE, "Data URI");
187+
} else {
188+
// Allow http/https only for non-data URIs
177189
::Microsoft::ReactNative::InputValidation::URLValidator::ValidateURL(uri, {"http", "https"});
178190
}
179191
} catch (const ::Microsoft::ReactNative::InputValidation::ValidationException &ex) {
@@ -192,8 +204,12 @@ void ImageLoader::prefetchImageWithMetadata(
192204
React::ReactPromise<bool> &&result) noexcept {
193205
// SDL Compliance: Validate URI for SSRF (P0 Critical - CVSS 7.8)
194206
try {
195-
// Allow data: URIs and http/https only
196-
if (uri.find("data:") != 0) {
207+
if (uri.find("data:") == 0) {
208+
// Validate data URI size to prevent DoS through memory exhaustion
209+
::Microsoft::ReactNative::InputValidation::SizeValidator::ValidateSize(
210+
uri.length(), ::Microsoft::ReactNative::InputValidation::SizeValidator::MAX_DATA_URI_SIZE, "Data URI");
211+
} else {
212+
// Allow http/https only for non-data URIs
197213
::Microsoft::ReactNative::InputValidation::URLValidator::ValidateURL(uri, {"http", "https"});
198214
}
199215
} catch (const ::Microsoft::ReactNative::InputValidation::ValidationException &ex) {

‎vnext/Microsoft.ReactNative/Modules/LinkingManagerModule.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ LinkingManager::~LinkingManager() noexcept {
5454
try {
5555
std::string urlUtf8 = Utf16ToUtf8(url);
5656
::Microsoft::ReactNative::InputValidation::URLValidator::ValidateURL(
57-
urlUtf8, {"http", "https", "mailto", "tel", "ms-settings"});
57+
urlUtf8, ::Microsoft::ReactNative::InputValidation::AllowedSchemes::LINKING_SCHEMES);
5858
} catch (const ::Microsoft::ReactNative::InputValidation::ValidationException &ex) {
5959
result.Reject(ex.what());
6060
co_return;
@@ -118,7 +118,7 @@ void LinkingManager::HandleOpenUri(winrt::hstring const &uri) noexcept {
118118
try {
119119
std::string uriUtf8 = winrt::to_string(uri);
120120
::Microsoft::ReactNative::InputValidation::URLValidator::ValidateURL(
121-
uriUtf8, {"http", "https", "mailto", "tel", "ms-settings"});
121+
uriUtf8, ::Microsoft::ReactNative::InputValidation::AllowedSchemes::LINKING_SCHEMES);
122122
} catch (const ::Microsoft::ReactNative::InputValidation::ValidationException &) {
123123
// Silently ignore invalid URIs to prevent crashes
124124
return;

‎vnext/Shared/Executors/WebSocketJSExecutor.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ void WebSocketJSExecutor::loadBundle(
8686
std::unique_ptr<const facebook::react::JSBigString> script,
8787
std::string sourceURL) {
8888
// SDL Compliance: Validate source URL (P1 - CVSS 5.5)
89+
// NOTE: 'file' scheme is allowed here because WebSocketJSExecutor is ONLY used in development/debugging scenarios.
90+
// This executor connects to Metro bundler during development and is never used in production builds.
91+
// Production apps use Hermes or Chakra with secure bundle loading that doesn't allow file:// URIs.
8992
try {
9093
if (!sourceURL.empty()) {
9194
Microsoft::ReactNative::InputValidation::URLValidator::ValidateURL(sourceURL, {"http", "https", "file"});

‎vnext/Shared/InputValidation.h‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,16 @@ class InvalidURLException : public std::logic_error {
4141
// Centralized allowlists for encodings
4242
namespace AllowedEncodings {
4343
static const std::vector<std::string> FILE_READER_ENCODINGS = {
44-
"UTF-8", "utf-8", "utf8",
45-
"UTF-16", "utf-16", "utf16",
46-
"ASCII", "ascii",
47-
"ISO-8859-1", "iso-8859-1",
44+
"UTF-8",
45+
"utf-8",
46+
"utf8",
47+
"UTF-16",
48+
"utf-16",
49+
"utf16",
50+
"ASCII",
51+
"ascii",
52+
"ISO-8859-1",
53+
"iso-8859-1",
4854
"" // Empty is allowed (defaults to UTF-8)
4955
};
5056
} // namespace AllowedEncodings
@@ -54,7 +60,7 @@ namespace AllowedSchemes {
5460
static const std::vector<std::string> HTTP_SCHEMES = {"http", "https"};
5561
static const std::vector<std::string> WEBSOCKET_SCHEMES = {"ws", "wss"};
5662
static const std::vector<std::string> FILE_SCHEMES = {"file"};
57-
static const std::vector<std::string> LINKING_SCHEMES = {"http", "https", "mailto", "tel"};
63+
static const std::vector<std::string> LINKING_SCHEMES = {"http", "https", "mailto", "tel", "ms-settings"};
5864
static const std::vector<std::string> IMAGE_SCHEMES = {"http", "https"};
5965
static const std::vector<std::string> DEBUG_SCHEMES = {"http", "https", "file"};
6066
} // namespace AllowedSchemes

‎vnext/Shared/Networking/WinRTHttpResource.cpp‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -324,11 +324,8 @@ void WinRTHttpResource::SendRequest(
324324
}
325325

326326
void WinRTHttpResource::AbortRequest(int64_t requestId) noexcept /*override*/ {
327-
// SDL Compliance: Validate request ID range (P2 - CVSS 3.5)
328-
try {
329-
Microsoft::ReactNative::InputValidation::SizeValidator::ValidateInt32Range(
330-
static_cast<int32_t>(requestId), 0, INT32_MAX, "Request ID");
331-
} catch (const Microsoft::ReactNative::InputValidation::ValidationException &) {
327+
// SDL Compliance: Validate request ID range BEFORE casting (P2 - CVSS 3.5)
328+
if (requestId < 0 || requestId > INT32_MAX) {
332329
// Invalid request ID, ignore abort
333330
return;
334331
}

0 commit comments

Comments
 (0)