Skip to content

Commit

Permalink
LibIMAP: Make parsing of atom data fallible
Browse files Browse the repository at this point in the history
We now return an error where `parse_atom()` would have previously
returned an empty StringView. This is consistent with RFC3501, which
says that an atom consists of one or more characters.

This prevents a few cases where parsing an invalid atom could lead to
an infinite loop.
  • Loading branch information
tcl3 authored and awesomekling committed Nov 8, 2023
1 parent b96a5f4 commit 4b99554
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 9 deletions.
18 changes: 10 additions & 8 deletions Userland/Libraries/LibIMAP/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ ErrorOr<void> Parser::parse_untagged()
auto number = try_parse_number();
if (number.has_value()) {
TRY(consume(" "sv));
auto data_type = parse_atom();
auto data_type = TRY(parse_atom());
if (data_type == "EXISTS"sv) {
m_response.data().set_exists(number.value());
TRY(consume("\r\n"sv));
Expand Down Expand Up @@ -180,7 +180,7 @@ ErrorOr<void> Parser::parse_untagged()
} else if (consume_if("OK"sv)) {
TRY(consume(" "sv));
if (consume_if("["sv)) {
auto actual_type = parse_atom();
auto actual_type = TRY(parse_atom());
if (actual_type == "CLOSED"sv) {
// No-op.
} else if (actual_type == "UIDNEXT"sv) {
Expand Down Expand Up @@ -230,7 +230,7 @@ ErrorOr<void> Parser::parse_untagged()
auto status_item = StatusItem();
status_item.set_mailbox(mailbox);
while (!consume_if(")"sv)) {
auto status_att = parse_atom();
auto status_att = TRY(parse_atom());
TRY(consume(" "sv));
auto value = TRY(parse_number());

Expand Down Expand Up @@ -582,14 +582,13 @@ ErrorOr<void> Parser::parse_capability_response()
auto capability = AK::Vector<DeprecatedString>();
while (!consume_if("\r\n"sv)) {
TRY(consume(" "sv));
auto x = DeprecatedString(parse_atom());
capability.append(x);
capability.append(TRY(parse_atom()));
}
m_response.data().add_capabilities(move(capability));
return {};
}

StringView Parser::parse_atom()
ErrorOr<StringView> Parser::parse_atom()
{
dbgln_if(IMAP_PARSER_DEBUG, "p: {}, parse_atom()", m_position);
auto is_non_atom_char = [](u8 x) {
Expand All @@ -604,14 +603,17 @@ StringView Parser::parse_atom()
m_position++;
}

if (count == 0)
return Error::from_string_literal("Invalid atom value");

StringView s = StringView(m_buffer.data() + start, count);
dbgln_if(IMAP_PARSER_DEBUG, "p: {}, ret \"{}\"", m_position, s);
return s;
}

ErrorOr<ResponseStatus> Parser::parse_status()
{
auto atom = parse_atom();
auto atom = TRY(parse_atom());

if (atom == "OK"sv) {
return ResponseStatus::OK;
Expand Down Expand Up @@ -733,7 +735,7 @@ ErrorOr<FetchCommand::DataItem> Parser::parse_fetch_data_item()
data_item.section->parts->append(num.value());
continue;
}
auto atom = parse_atom();
auto atom = TRY(parse_atom());
if (atom.equals_ignoring_ascii_case("MIME"sv)) {
data_item.section->ends_with_mime = true;
continue;
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibIMAP/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Parser {
ErrorOr<void> parse_untagged();
ErrorOr<void> parse_capability_response();

StringView parse_atom();
ErrorOr<StringView> parse_atom();
ErrorOr<StringView> parse_quoted_string();
ErrorOr<StringView> parse_literal_string();
ErrorOr<StringView> parse_string();
Expand Down

0 comments on commit 4b99554

Please sign in to comment.