header_rewrite: Parser::preprocess() indexes tokens[0] after pop_back() can empty the vector
- Dominant language
- C++
- Stars
- 2k
- Forks
- 874
- Avg merge
- 6d 15h
- Merged PRs (30d)
- 46
Description
## Summary
`Parser::preprocess()` in header_rewrite can pop the only token off the vector and then index `tokens[0]` without checking whether anything is left.
## Detail
[`plugins/header_rewrite/parser.cc:194`](https://github.com/apache/trafficserver/blob/master/plugins/header_rewrite/parser.cc#L194) consumes a trailing flags section:
```cpp
if (tokens.size() > 0) {
std::string m = tokens[tokens.size() - 1];
if (!m.empty() && (m[0] == '[')) {
if (m[m.size() - 1] == ']') {
...
tokens.pop_back(); // consume it, so we don't concatenate it into the value
} else {
...
}
}
}
// Special case for "conditional" values
if (tokens[0].substr(0, 2) == "%{") {
```
A configuration line whose only token is a flags section, `[L]` on a line by itself for instance, gives `tokens.size() == 1`. The `pop_back()` empties the vector, and the very next statement indexes `tokens[0]`.
`std::vector::operator[]` does no bounds checking, so this is an out-of-bounds read on a configuration file that a user can write.
## Proposed fix
An `if (tokens.empty())` guard after the `pop_back()`, returning `false` with a `TSError` describing the offending line. A flags-only line is not a valid rule, so rejecting it with a message beats reading past the end of the vector.
## Context
Pre-existing, and adjacent to a hunk in [PR #13591](https://github.com/apache/trafficserver/pull/13591). Filing it separately to keep that PR purely mechanical.
Contributor guide
Assessment
This issue has not been assessed yet.