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@-1 {{expected expression}}
+// expected-error@-2 {{expected ']'}}
+// expected-note@-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();
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@-1 {{expected expression}}
+// expected-error@-2 {{expected ']'}}
+// expected-note@-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();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
