Commit 71b9018c authored by jgruber's avatar jgruber Committed by Commit Bot

[regexp] Avoid integer overflow in callable @@replace

The integer value denoting the number of captures (and thus the size
of the list of captures created in @@replace [0]) can be controlled by
the user.  This CL ensures we don't overflow and respect
Code::kMaxArguments, but note that it is still possible to trigger
OOMs through large lists.

Bug: chromium:786573
Change-Id: I19c88908c594487818d083b2ba423764ef91eae0
Reviewed-on: https://chromium-review.googlesource.com/779001Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49530}
parent da82b257
......@@ -21,6 +21,20 @@ namespace internal {
namespace {
// Returns -1 for failure.
uint32_t GetArgcForReplaceCallable(uint32_t num_captures,
bool has_named_captures) {
const uint32_t kAdditionalArgsWithoutNamedCaptures = 2;
const uint32_t kAdditionalArgsWithNamedCaptures = 3;
if (num_captures > Code::kMaxArguments) return -1;
uint32_t argc = has_named_captures
? num_captures + kAdditionalArgsWithNamedCaptures
: num_captures + kAdditionalArgsWithoutNamedCaptures;
STATIC_ASSERT(Code::kMaxArguments < std::numeric_limits<uint32_t>::max() -
kAdditionalArgsWithNamedCaptures);
return (argc > Code::kMaxArguments) ? -1 : argc;
}
// Looks up the capture of the given name. Returns the (1-based) numbered
// capture index or -1 on failure.
int LookupNamedCapture(std::function<bool(String*)> name_matches,
......@@ -1483,7 +1497,11 @@ RUNTIME_FUNCTION(Runtime_StringReplaceNonGlobalRegExpWithFunction) {
}
DCHECK_IMPLIES(has_named_captures, FLAG_harmony_regexp_named_captures);
const int argc = has_named_captures ? m + 3 : m + 2;
const uint32_t argc = GetArgcForReplaceCallable(m, has_named_captures);
if (argc == static_cast<uint32_t>(-1)) {
THROW_NEW_ERROR_RETURN_FAILURE(
isolate, NewRangeError(MessageTemplate::kTooManyArguments));
}
ScopedVector<Handle<Object>> argv(argc);
int cursor = 0;
......@@ -1794,7 +1812,8 @@ RUNTIME_FUNCTION(Runtime_RegExpReplace) {
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, captures_length_obj,
Object::ToLength(isolate, captures_length_obj));
const int captures_length = PositiveNumberToUint32(*captures_length_obj);
const uint32_t captures_length =
PositiveNumberToUint32(*captures_length_obj);
Handle<Object> match_obj;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, match_obj,
......@@ -1819,7 +1838,7 @@ RUNTIME_FUNCTION(Runtime_RegExpReplace) {
// Do not reserve capacity since captures_length is user-controlled.
ZoneVector<Handle<Object>> captures(&zone);
for (int n = 0; n < captures_length; n++) {
for (uint32_t n = 0; n < captures_length; n++) {
Handle<Object> capture;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, capture, Object::GetElement(isolate, result, n));
......@@ -1843,12 +1862,17 @@ RUNTIME_FUNCTION(Runtime_RegExpReplace) {
Handle<String> replacement;
if (functional_replace) {
const int argc =
has_named_captures ? captures_length + 3 : captures_length + 2;
const uint32_t argc =
GetArgcForReplaceCallable(captures_length, has_named_captures);
if (argc == static_cast<uint32_t>(-1)) {
THROW_NEW_ERROR_RETURN_FAILURE(
isolate, NewRangeError(MessageTemplate::kTooManyArguments));
}
ScopedVector<Handle<Object>> argv(argc);
int cursor = 0;
for (int j = 0; j < captures_length; j++) {
for (uint32_t j = 0; j < captures_length; j++) {
argv[cursor++] = captures[j];
}
......
......@@ -5,7 +5,7 @@
let i = 0;
let re = /./g;
re.exec = () => {
if (i++ == 0) return { length: 2147483648 };
if (i++ == 0) return { length: 2 ** 16 };
return null;
};
......
// Copyright 2017 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
let cnt = 0;
let reg = /./g;
reg.exec = () => {
// Note: it's still possible to trigger OOM by passing huge values here, since
// the spec requires building a list of all captures in
// https://tc39.github.io/ecma262/#sec-regexp.prototype-@@replace
if (cnt++ == 0) return {length: 2 ** 16};
cnt = 0;
return null;
};
assertThrows(() => ''.replace(reg, () => {}), RangeError);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment