commit d6131075f75ba4e7ae1ba62a5f4a57eb7f6122ba
author Scott Graham <scottmg@chromium.org> Mon Apr 06 14:05:42 2020 -0700
committer Commit Bot <commit-bot@chromium.org> Tue Apr 07 00:03:08 2020 +0000
tree 1f941fd1f6e1147cbd072f3031cbf7d708a4679d
parent a38ba1c8be24c021781e3939249d88cb05f58a13

format: Don't crash on non-literal import

import() with non-literal as argument was crashing in attempting to sort
them during format. There's no much intelligent that we can do to sort
for example a function call in import() so just treat them as empty so
they go before the literal ones. A stable sort guarantees that as a
group the relative order stays the same.

Crash found by afl-fuzz.

Bug: gn:156
Change-Id: If38d4d81a20bc90f28dd60332e3bb66206c0293e
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/7980
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>

src/gn/command_format.cc

diff --git a/src/gn/command_format.cc b/src/gn/command_format.cc
index d475893..67cd78b 100644
--- a/src/gn/command_format.cc
+++ b/src/gn/command_format.cc
@@ -443,9 +443,14 @@
       const auto& b_args = b->AsFunctionCall()->args()->contents();
       std::string_view a_name;
       std::string_view b_name;
-      if (!a_args.empty())
+
+      // Non-literal imports are treated as empty names, and order is
+      // maintained. Arbitrarily complex expressions in import() are
+      // rare, and it probably doesn't make sense to sort non-string
+      // literals anyway, see format_test_data/083.gn.
+      if (!a_args.empty() && a_args[0]->AsLiteral())
         a_name = a_args[0]->AsLiteral()->value().value();
-      if (!b_args.empty())
+      if (!b_args.empty() && b_args[0]->AsLiteral())
         b_name = b_args[0]->AsLiteral()->value().value();
 
       auto is_absolute = [](std::string_view import) {

src/gn/command_format_unittest.cc

diff --git a/src/gn/command_format_unittest.cc b/src/gn/command_format_unittest.cc
index 4dea55e..2141f6f 100644
--- a/src/gn/command_format_unittest.cc
+++ b/src/gn/command_format_unittest.cc
@@ -127,3 +127,4 @@
 FORMAT_TEST(080)
 FORMAT_TEST(081)
 FORMAT_TEST(082)
+FORMAT_TEST(083)

src/gn/format_test_data/083.gn

diff --git a/src/gn/format_test_data/083.gn b/src/gn/format_test_data/083.gn
new file mode 100644
index 0000000..6936bf5
--- /dev/null
+++ b/src/gn/format_test_data/083.gn
@@ -0,0 +1,7 @@
+# Crash found by afl-fuzz (non-literal import).
+import("x")
+import(sources)
+import(zip)
+import(zap)
+import(a+b+c+d)
+import(zzz+yyy)

src/gn/format_test_data/083.golden

diff --git a/src/gn/format_test_data/083.golden b/src/gn/format_test_data/083.golden
new file mode 100644
index 0000000..445534d
--- /dev/null
+++ b/src/gn/format_test_data/083.golden
@@ -0,0 +1,7 @@
+# Crash found by afl-fuzz (non-literal import).
+import(sources)
+import(zip)
+import(zap)
+import(a + b + c + d)
+import(zzz + yyy)
+import("x")