From cf05e4ca792ea82f01c06bed8ec671db198cdf22 Mon Sep 17 00:00:00 2001
From: Michael Achenbach <machenbach@chromium.org>
Date: Mon, 10 Feb 2020 14:29:33 +0100
Subject: [PATCH] Make using natives for fuzzing more permissive

This makes creating whitelisted runtime functions more permissive on
fuzzers (when --allow-natives-for-fuzzing is passed).

- Runtime functions with too few arguments are replaced with undefined.
- Superfluous arguments are ignored.

This reduces syntax-error rate on fuzzers. Also prevents
dcheck errors when fuzzing debug builds and fuzzers use too many
arguments for runtime functions.

Bug: chromium:1044942
Change-Id: I23b45398421c50bc82d1e8bfdf019f565253db96
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2039352
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66202}
---
 src/parsing/parser.cc                  | 44 +++++++++++++++++++-------
 src/parsing/parser.h                   |  4 +++
 test/mjsunit/call-intrinsic-fuzzing.js |  9 +++++-
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index 31cc3a8c17..eb890bface 100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -356,17 +356,16 @@ Expression* Parser::NewV8Intrinsic(const AstRawString* name,
   const Runtime::Function* function =
       Runtime::FunctionForName(name->raw_data(), name->length());
 
+  // Be more premissive when fuzzing. Intrinsics are not supported.
+  if (FLAG_allow_natives_for_fuzzing) {
+    return NewV8RuntimeFunctionForFuzzing(function, args, pos);
+  }
+
   if (function != nullptr) {
     // Check for possible name clash.
     DCHECK_EQ(Context::kNotFound,
               Context::IntrinsicIndexForName(name->raw_data(), name->length()));
 
-    // When fuzzing, only allow whitelisted runtime functions.
-    if (FLAG_allow_natives_for_fuzzing &&
-        !Runtime::IsWhitelistedForFuzzing(function->function_id)) {
-      return factory()->NewUndefinedLiteral(kNoSourcePosition);
-    }
-
     // Check that the expected number of arguments are being passed.
     if (function->nargs != -1 && function->nargs != args.length()) {
       ReportMessage(MessageTemplate::kRuntimeWrongNumArgs);
@@ -376,11 +375,6 @@ Expression* Parser::NewV8Intrinsic(const AstRawString* name,
     return factory()->NewCallRuntime(function, args, pos);
   }
 
-  // Intrinsics are not supported for fuzzing.
-  if (FLAG_allow_natives_for_fuzzing) {
-    return factory()->NewUndefinedLiteral(kNoSourcePosition);
-  }
-
   int context_index =
       Context::IntrinsicIndexForName(name->raw_data(), name->length());
 
@@ -393,6 +387,34 @@ Expression* Parser::NewV8Intrinsic(const AstRawString* name,
   return factory()->NewCallRuntime(context_index, args, pos);
 }
 
+// More permissive runtime-function creation on fuzzers.
+Expression* Parser::NewV8RuntimeFunctionForFuzzing(
+    const Runtime::Function* function, const ScopedPtrList<Expression>& args,
+    int pos) {
+  CHECK(FLAG_allow_natives_for_fuzzing);
+
+  // Intrinsics are not supported for fuzzing. Only allow whitelisted runtime
+  // functions. Also prevent later errors due to too few arguments and just
+  // ignore this call.
+  if (function == nullptr ||
+      !Runtime::IsWhitelistedForFuzzing(function->function_id) ||
+      function->nargs > args.length()) {
+    return factory()->NewUndefinedLiteral(kNoSourcePosition);
+  }
+
+  // Flexible number of arguments permitted.
+  if (function->nargs == -1) {
+    return factory()->NewCallRuntime(function, args, pos);
+  }
+
+  // Otherwise ignore superfluous arguments.
+  ScopedPtrList<Expression> permissive_args(pointer_buffer());
+  for (int i = 0; i < function->nargs; i++) {
+    permissive_args.Add(args.at(i));
+  }
+  return factory()->NewCallRuntime(function, permissive_args, pos);
+}
+
 Parser::Parser(ParseInfo* info)
     : ParserBase<Parser>(info->zone(), &scanner_, info->stack_limit(),
                          info->extension(), info->GetOrCreateAstValueFactory(),
diff --git a/src/parsing/parser.h b/src/parsing/parser.h
index d53ccc7fbd..cea5104512 100644
--- a/src/parsing/parser.h
+++ b/src/parsing/parser.h
@@ -833,6 +833,10 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   Expression* NewV8Intrinsic(const AstRawString* name,
                              const ScopedPtrList<Expression>& args, int pos);
 
+  Expression* NewV8RuntimeFunctionForFuzzing(
+      const Runtime::Function* function, const ScopedPtrList<Expression>& args,
+      int pos);
+
   V8_INLINE Statement* NewThrowStatement(Expression* exception, int pos) {
     return factory()->NewExpressionStatement(
         factory()->NewThrow(exception, pos), pos);
diff --git a/test/mjsunit/call-intrinsic-fuzzing.js b/test/mjsunit/call-intrinsic-fuzzing.js
index 44c3f59104..3945c8d49d 100644
--- a/test/mjsunit/call-intrinsic-fuzzing.js
+++ b/test/mjsunit/call-intrinsic-fuzzing.js
@@ -10,10 +10,17 @@
 assertEquals(undefined, %GetOptimizationStatus(function (){}));
 
 // Blacklisted intrinsics can have wrong arguments.
-%GetOptimizationStatus(1, 2, 3, 4);
+assertEquals(undefined, %GetOptimizationStatus(1, 2, 3, 4));
 
 // We don't care if an intrinsic actually exists.
 assertEquals(undefined, %FooBar());
 
 // Check whitelisted intrinsic.
 assertNotEquals(undefined, %IsBeingInterpreted());
+
+// Whitelisted runtime functions with too few args are ignored.
+assertEquals(undefined, %DeoptimizeFunction());
+
+// Superfluous arguments are ignored.
+%DeoptimizeFunction(function() {}, undefined);
+assertNotEquals(undefined, %IsBeingInterpreted(1, 2, 3));