[PATCH] D16572: PR23057: fix use-after-free due to local token buffer in ParseCXXAmbiguousParenExpression

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin created this revision.
DmitryPolukhin added a reviewer: rjmccall.
DmitryPolukhin added a subscriber: cfe-commits.

To completely eliminate use-after-free in this place I had to copy tokens into new array and pass ownership. As far as I understand the code it is not possible to guarantee that all inserted tokens are consumed before exiting from this function. Depending on how many tokens were consumed before SkipUntil is called, it may not happen due to unbalanced brackets, etc. Other places where EnterTokenStream is called usually pass vector that some class owns but here it is not possible because this place can be called recursively.

http://reviews.llvm.org/D16572

Files:
  lib/Parse/ParseExprCXX.cpp
  test/Parser/cxx-ambig-paren-expr-asan.cpp

Index: test/Parser/cxx-ambig-paren-expr-asan.cpp
===================================================================
--- /dev/null
+++ test/Parser/cxx-ambig-paren-expr-asan.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
+
+// This syntax error used to cause use-after free due to token local buffer
+// in ParseCXXAmbiguousParenExpression.
+int H((int()[)]);
+// expected-error at -1 {{expected expression}}
+// expected-error at -2 {{expected ']'}}
+// expected-note at -3 {{to match this '['}}
Index: lib/Parse/ParseExprCXX.cpp
===================================================================
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -3083,10 +3083,17 @@
 
   // The current token should go after the cached tokens.
   Toks.push_back(Tok);
+
+  // Make a copy of stored token to pass ownership to EnterTokenStream function.
+  // This copy is required because we may exit from this function when some
+  // tokens are not consumed yet.
+  Token *Buffer = new Token[Toks.size()];
+  std::copy(Toks.begin(), Toks.end(), Buffer);
+
   // Re-enter the stored parenthesized tokens into the token stream, so we may
   // parse them now.
-  PP.EnterTokenStream(Toks.data(), Toks.size(),
-                      true/*DisableMacroExpansion*/, false/*OwnsTokens*/);
+  PP.EnterTokenStream(Buffer, Toks.size(),
+                      true/*DisableMacroExpansion*/, true/*OwnsTokens*/);
   // Drop the current token and bring the first cached one. It's the same token
   // as when we entered this function.
   ConsumeAnyToken();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16572.45960.patch
Type: text/x-patch
Size: 1566 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160126/50c6a719/attachment.bin>