[Security] Multiple Memory Safety and DoS Vulnerabilities in robots.cc
- Dominant language
- C++
- Stars
- 3.5k
- Forks
- 255
- PR merge metrics
- No merged PRs in 30d
Description
### Summary
I identified several issues in the `google/robotstxt` library during fuzzing and manual code review. These observations involve URL matching consistency, algorithmic complexity, boundary handling, and integer arithmetic robustness within `robots.cc`.
The most significant finding is an inconsistency in how percent-encoded paths are normalized and matched. This can lead to different policy decisions for semantically equivalent URLs.
These findings were previously shared with the Google OSS VRP, and I was advised to open an issue here for project-level review and discussion.
---
### Observations & Analysis
#### 1. Inconsistent Percent-Encoding Matching
**Detail**
`MaybeEscapePattern()` normalizes lowercase percent-encoded sequences appearing in `robots.txt` rules (e.g. `%aa` → `%AA`).
However, the URI path extracted by `GetPathParamsQuery()` does not appear to undergo equivalent normalization before matching.
As a result, semantically equivalent percent-encoded representations may produce different matching outcomes because matching is performed using case-sensitive comparisons.
**Example**
Rule:
```text
Disallow: /secret%aa
```
Internal representation:
```text
/secret%AA
```
URI:
```text
/secret%aa
```
Observed result:
```text
ALLOWED
```
Expected result:
```text
DISALLOWED
```
**Impact**
This is not an authorization bypass in the traditional sense because `robots.txt` is not an authorization mechanism.
However, it appears to cause inconsistent policy evaluation where equivalent URL representations can lead to different crawler decisions. Applications relying on `robotstxt` matching for crawler restrictions or policy enforcement may observe unintended behavior.
---
#### 2. Algorithmic Complexity in `Matches`
**Detail**
The matching algorithm exhibits `O(N × M)` worst-case behavior.
Specific combinations of long paths and wildcard-heavy patterns (for example `*a*a*a...`) can cause significant growth in processing time.
**Measurements**
| Path Length | Pattern Size | Time |
|------------|------------|------|
| 10,000 | 200 | 0.05s |
| 80,000 | 1600 | 3.73s |
**Impact**
This may lead to excessive CPU consumption in high-throughput parsing environments.
---
#### 3. Boundary Handling in `ExtractUserAgent`
**Detail**
`ExtractUserAgent()` iterates through a `string_view` without an explicit `p < end` boundary check.
While normal usage may provide safe backing storage, the parser can advance beyond the logical range represented by the `string_view`.
**Impact**
This is primarily a robustness and defensive-programming concern. Adding explicit boundary validation would ensure that parsing remains constrained to the intended slice.
---
#### 4. Integer Arithmetic Robustness in `Matches`
**Detail**
`numpos` is stored as a 32-bit `int`.
Calculations derived from path length may overflow for sufficiently large inputs, potentially resulting in unexpected behavior.
**Impact**
Although extremely large paths are uncommon, replacing path-length-derived counters with `size_t` would improve resilience and eliminate this edge case.
---
### Reproduction (Observation #1)
The matching inconsistency can be reproduced using the bundled CLI tool.
```bash
# Create robots.txt
echo -e "User-agent: *\nDisallow: /secret%aa" > robots.txt
# Run matcher
./build/robots robots.txt FooBot "http://example.com/secret%aa"
```
Observed:
```text
ALLOWED
```
Expected:
```text
DISALLOWED
```
---
### Suggested Fixes
#### Fix 1: Consistent Percent-Encoding Normalization
Apply the same percent-encoding normalization logic to incoming URI paths before matching.
#### Fix 2: Complexity Safeguards
Consider introducing a complexity threshold, state limit, or other safeguards for wildcard-heavy patterns.
#### Fix 3: Boundary Validation
Add explicit bounds checking in `ExtractUserAgent()`:
```cpp
while (p < end && ...)
```
#### Fix 4: Type Safety
Use `size_t` for path-length-derived counters and indices in `Matches()`.
Contributor guide
Research direction
Start with robots.cc and reproduce the percent-encoding case using the bundled CLI command in the issue. Read MaybeEscapePattern(), GetPathParamsQuery(), Matches(), and ExtractUserAgent() to verify each observation separately. The issue is done only after the project agrees on scope and confirms fixes for the selected findings with regression coverage and bounded behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100