From e3ec759f97eb1fcda8ce48c8d4fec2f8a3513c87 Mon Sep 17 00:00:00 2001 From: AnotherTest Date: Tue, 2 Feb 2021 20:20:05 +0330 Subject: [PATCH] Shell: Make history range values larger than u32 a syntax error Found by oss-fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29792&sort=reported&q=serenity --- Userland/Shell/AST.cpp | 4 ++++ Userland/Shell/AST.h | 1 + Userland/Shell/Parser.cpp | 28 +++++++++++++++++++++++----- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/Userland/Shell/AST.cpp b/Userland/Shell/AST.cpp index d4ff8ecd9f..eeeaac98d3 100644 --- a/Userland/Shell/AST.cpp +++ b/Userland/Shell/AST.cpp @@ -1364,6 +1364,10 @@ HistoryEvent::HistoryEvent(Position position, HistorySelector selector) : Node(move(position)) , m_selector(move(selector)) { + if (m_selector.word_selector_range.start.syntax_error_node) + set_is_syntax_error(*m_selector.word_selector_range.start.syntax_error_node); + else if (m_selector.word_selector_range.end.has_value() && m_selector.word_selector_range.end->syntax_error_node) + set_is_syntax_error(*m_selector.word_selector_range.end->syntax_error_node); } HistoryEvent::~HistoryEvent() diff --git a/Userland/Shell/AST.h b/Userland/Shell/AST.h index 5d34c49173..b0f4e7d5f8 100644 --- a/Userland/Shell/AST.h +++ b/Userland/Shell/AST.h @@ -908,6 +908,7 @@ struct HistorySelector { WordSelectorKind kind { Index }; size_t selector { 0 }; Position position; + RefPtr syntax_error_node; size_t resolve(size_t size) const { diff --git a/Userland/Shell/Parser.cpp b/Userland/Shell/Parser.cpp index d705a9a27a..44127b5f77 100644 --- a/Userland/Shell/Parser.cpp +++ b/Userland/Shell/Parser.cpp @@ -1377,9 +1377,16 @@ RefPtr Parser::parse_history_designator() selector.event.kind = AST::HistorySelector::EventKind::StartingStringLookup; selector.event.text_position = { m_offset, m_offset, m_line, m_line }; selector.word_selector_range = { - { AST::HistorySelector::WordSelectorKind::Index, 0, { m_offset, m_offset, m_line, m_line } }, AST::HistorySelector::WordSelector { - AST::HistorySelector::WordSelectorKind::Last, 0, { m_offset, m_offset, m_line, m_line } }, + AST::HistorySelector::WordSelectorKind::Index, + 0, + { m_offset, m_offset, m_line, m_line }, + nullptr }, + AST::HistorySelector::WordSelector { + AST::HistorySelector::WordSelectorKind::Last, + 0, + { m_offset, m_offset, m_line, m_line }, + nullptr } }; switch (peek()) { @@ -1433,10 +1440,19 @@ RefPtr Parser::parse_history_designator() if (isdigit(c)) { auto num = consume_while(is_digit); auto value = num.to_uint(); + if (!value.has_value()) { + return AST::HistorySelector::WordSelector { + AST::HistorySelector::WordSelectorKind::Index, + 0, + { m_rule_start_offsets.last(), m_offset, m_rule_start_lines.last(), line() }, + create("Word selector value invalid or out of range") + }; + } return AST::HistorySelector::WordSelector { AST::HistorySelector::WordSelectorKind::Index, value.value(), - { m_rule_start_offsets.last(), m_offset, m_rule_start_lines.last(), line() } + { m_rule_start_offsets.last(), m_offset, m_rule_start_lines.last(), line() }, + nullptr }; } if (c == '^') { @@ -1444,7 +1460,8 @@ RefPtr Parser::parse_history_designator() return AST::HistorySelector::WordSelector { AST::HistorySelector::WordSelectorKind::Index, 0, - { m_rule_start_offsets.last(), m_offset, m_rule_start_lines.last(), line() } + { m_rule_start_offsets.last(), m_offset, m_rule_start_lines.last(), line() }, + nullptr }; } if (c == '$') { @@ -1452,7 +1469,8 @@ RefPtr Parser::parse_history_designator() return AST::HistorySelector::WordSelector { AST::HistorySelector::WordSelectorKind::Last, 0, - { m_rule_start_offsets.last(), m_offset, m_rule_start_lines.last(), line() } + { m_rule_start_offsets.last(), m_offset, m_rule_start_lines.last(), line() }, + nullptr }; } return {};