Skip to content

Commit 715132c

Browse files
committed
fix(parse): fix use-after-free after rolling back transaction
Rolling back a transaction queues memory to be rewound. This is incorrect if the transaction object is destructed later. This leads to a use-after-free bug with the following input: await (0scribeBlock => { switch (child.type) { case 'test': { } } async describeBlock => { await (x) => { await (0scribeBlock => { await (x) => { await (0s => { @ @ @; @ @ @; The specific cause is the following code pattern in parser::parse_await_expression: { parser_transaction transaction = this->begin_transaction(); // ... this->roll_back_transaction(std::move(transaction)); this->diag_reporter_->report(diag_await_followed_by_arrow_function{ .await_operator = operator_span, }); // (a) } // (b) Line (a) allocates memory in the to-be-unwound allocator. Line (b) performs the rewind. After (b), reading from diag_reporter_ (which in the buggy case is a buffering_diag_reporter) leads to a use-after-free error. Fix the problem by having roll_back_transaction rewind immediately, instead of deferring rewind to an arbitrary point in the future. Fixing this issue exposed another use-after-free bug, causing test_lex.rolled_back_outer_transaction_discards_errors to fail in ASAN builds. Fix this by promptly destructing the buffering_diag_reporter when committing a transaction. Ref: 51a2ba7
1 parent a03b16c commit 715132c

File tree

3 files changed

+18
-7
lines changed

3 files changed

+18
-7
lines changed

src/lex.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1128,14 +1128,18 @@ void lexer::commit_transaction(lexer_transaction&& transaction) {
11281128
buffered_diagnostics->move_into(transaction.old_diag_reporter);
11291129

11301130
this->diag_reporter_ = transaction.old_diag_reporter;
1131+
1132+
transaction.reporter.reset();
11311133
}
11321134

11331135
void lexer::roll_back_transaction(lexer_transaction&& transaction) {
11341136
this->last_token_ = transaction.old_last_token;
11351137
this->last_last_token_end_ = transaction.old_last_last_token_end;
11361138
this->input_ = transaction.old_input;
11371139
this->diag_reporter_ = transaction.old_diag_reporter;
1138-
transaction.allocator_rewind.rewind_on_destruct();
1140+
1141+
transaction.reporter.reset();
1142+
this->transaction_allocator_.rewind(std::move(transaction.allocator_rewind));
11391143
}
11401144

11411145
bool lexer::transaction_has_lex_diagnostics(const lexer_transaction&) const

src/quick-lint-js/lex.h

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <quick-lint-js/location.h>
1515
#include <quick-lint-js/monotonic-allocator.h>
1616
#include <quick-lint-js/narrow-cast.h>
17+
#include <quick-lint-js/optional.h>
1718
#include <quick-lint-js/padded-string.h>
1819
#include <quick-lint-js/token.h>
1920
#include <quick-lint-js/utf-8.h>
@@ -316,27 +317,28 @@ struct lexer_transaction {
316317
const char8* old_input,
317318
diag_reporter** diag_reporter_pointer,
318319
allocator_type* allocator)
319-
: allocator_rewind(allocator),
320+
: allocator_rewind(allocator->prepare_for_rewind()),
320321
old_last_token(old_last_token),
321322
old_last_last_token_end(old_last_last_token_end),
322323
old_input(old_input),
323-
reporter(allocator),
324+
reporter(std::in_place, allocator),
324325
old_diag_reporter(
325-
std::exchange(*diag_reporter_pointer, &this->reporter)) {}
326+
std::exchange(*diag_reporter_pointer, get(this->reporter))) {}
326327

327328
// Don't allow copying a transaction. lexer::diag_reporter_ might point to
328329
// lexer_transaction::diag_reporter.
329330
lexer_transaction(const lexer_transaction&) = delete;
330331
lexer_transaction& operator=(const lexer_transaction&) = delete;
331332

332333
// Rewinds memory allocated by 'reporter'. Must be constructed before
333-
// 'reporter' (thus destructed after 'reporter').
334-
allocator_type::conditional_rewind_guard allocator_rewind;
334+
// 'reporter' is constructed. 'allocator_type::rewind' must be called before
335+
// 'reporter' is destructed.
336+
allocator_type::rewind_state allocator_rewind;
335337

336338
token old_last_token;
337339
const char8* old_last_last_token_end;
338340
const char8* old_input;
339-
buffering_diag_reporter reporter;
341+
std::optional<buffering_diag_reporter> reporter;
340342
diag_reporter* old_diag_reporter;
341343
};
342344
}

src/quick-lint-js/optional.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@
77
#include <optional>
88

99
namespace quick_lint_js {
10+
template <class T>
11+
T *get(std::optional<T> &o) noexcept {
12+
return o.has_value() ? &*o : nullptr;
13+
}
14+
1015
template <class T>
1116
const T *get(const std::optional<T> &o) noexcept {
1217
return o.has_value() ? &*o : nullptr;

0 commit comments

Comments
 (0)