Skip to content

Commit

Permalink
fix(flux-core): infinite loop protection in the parser (#5436)
Browse files Browse the repository at this point in the history
Update all loops using self.more in the parser to detect if they
get stuck attempting to process the same token multiple times. This
has been observed to cause the parser to get into an infinite loop
with some erroneus inputs.

The protection code was copied from the parse_array_items_rest and
applied everywhere the parser could conceivably get stuck. It is
not clear that it is possible to get stuck in all the places that
the protection was added.
  • Loading branch information
mhilton authored Oct 25, 2023
1 parent a5f310a commit 43ad07e
Show file tree
Hide file tree
Showing 6 changed files with 790 additions and 1 deletion.
77 changes: 77 additions & 0 deletions libflux/flux-core/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -734,12 +734,23 @@ impl<'input> Parser<'input> {
// Parameters = Parameter { "," Parameter } .
fn parse_parameters(&mut self) -> Vec<ParameterType> {
let mut params = Vec::<ParameterType>::new();
// keep track of the last token's byte offsets
let mut last = self.peek().start_offset;
while self.more() {
let parameter = self.parse_parameter_type();
params.push(parameter);
if self.peek().tok == TokenType::Comma {
self.consume();
}

// If we parse the same token twice in a row,
// it means we've hit a parse error, and that
// we're now in an infinite loop.
let this = self.peek().start_offset;
if last == this {
break;
}
last = this;
}
params
}
Expand Down Expand Up @@ -889,11 +900,22 @@ impl<'input> Parser<'input> {
self.consume();
}
// check for more properties
// keep track of the last token's byte offsets
let mut last = self.peek().start_offset;
while self.more() {
properties.push(self.parse_property_type());
if self.peek().tok == TokenType::Comma {
self.consume();
}

// If we parse the same token twice in a row,
// it means we've hit a parse error, and that
// we're now in an infinite loop.
let this = self.peek().start_offset;
if last == this {
break;
}
last = this;
}
properties
}
Expand Down Expand Up @@ -1928,6 +1950,8 @@ impl<'input> Parser<'input> {
val,
comma: comma.comments,
}];
// keep track of the last token's byte offsets
let mut last = self.peek().start_offset;
while self.more() {
let key = self.parse_expression();
self.expect(TokenType::Colon);
Expand All @@ -1940,6 +1964,15 @@ impl<'input> Parser<'input> {
_ => vec![],
};
items.push(DictItem { key, val, comma });

// If we parse the same token twice in a row,
// it means we've hit a parse error, and that
// we're now in an infinite loop.
let this = self.peek().start_offset;
if last == this {
break;
}
last = this;
}
let end = self.close(TokenType::RBrack);
Expression::Dict(Box::new(DictExpr {
Expand Down Expand Up @@ -2066,6 +2099,8 @@ impl<'input> Parser<'input> {
}
_ => {
let mut expr = self.parse_expression_suffix(Expression::Identifier(key));
// keep track of the last token's byte offsets
let mut last = self.peek().start_offset;
while self.more() {
let rhs = self.parse_expression();
if let Expression::Bad(_) = rhs {
Expand All @@ -2084,6 +2119,15 @@ impl<'input> Parser<'input> {
left: expr,
right: rhs,
}));

// If we parse the same token twice in a row,
// it means we've hit a parse error, and that
// we're now in an infinite loop.
let this = self.peek().start_offset;
if last == this {
break;
}
last = this;
}
let rparen = self.close(TokenType::RParen);
Expression::Paren(Box::new(ParenExpr {
Expand Down Expand Up @@ -2180,6 +2224,8 @@ impl<'input> Parser<'input> {
fn parse_property_list(&mut self) -> Vec<Property> {
let mut params = Vec::new();
let mut errs = Vec::new();
// keep track of the last token's byte offsets
let mut last = self.peek().start_offset;
while self.more() {
let t = self.peek();
let mut p: Property = match t.tok {
Expand All @@ -2198,6 +2244,15 @@ impl<'input> Parser<'input> {
}

params.push(p);

// If we parse the same token twice in a row,
// it means we've hit a parse error, and that
// we're now in an infinite loop.
let this = self.peek().start_offset;
if last == this {
break;
}
last = this;
}
self.errs.append(&mut errs);
params
Expand Down Expand Up @@ -2285,13 +2340,24 @@ impl<'input> Parser<'input> {
}
fn parse_parameter_list(&mut self) -> Vec<Property> {
let mut params = Vec::new();
// keep track of the last token's byte offsets
let mut last = self.peek().start_offset;
while self.more() {
let mut p = self.parse_parameter();
if self.peek().tok == TokenType::Comma {
let t = self.scan();
p.comma = t.comments;
};
params.push(p);

// If we parse the same token twice in a row,
// it means we've hit a parse error, and that
// we're now in an infinite loop.
let this = self.peek().start_offset;
if last == this {
break;
}
last = this;
}
params
}
Expand Down Expand Up @@ -2394,6 +2460,8 @@ impl<'input> Parser<'input> {
fn parse_attribute_params(&mut self) -> Vec<AttributeParam> {
let mut params = Vec::new();
let mut errs = Vec::new();
// keep track of the last token's byte offsets
let mut last = self.peek().start_offset;
while self.more() {
let value = self.parse_primary_expression();
let start_pos = value.base().location.start;
Expand All @@ -2420,6 +2488,15 @@ impl<'input> Parser<'input> {
comma: comments,
};
params.push(param);

// If we parse the same token twice in a row,
// it means we've hit a parse error, and that
// we're now in an infinite loop.
let this = self.peek().start_offset;
if last == this {
break;
}
last = this;
}
self.errs.append(&mut errs);
params
Expand Down
Loading

0 comments on commit 43ad07e

Please sign in to comment.