bujjibabukatta opened a new pull request, #70716:
URL: https://github.com/apache/airflow/pull/70716
## Summary
`SparkSubmitHook._mask_cmd()` is used to redact secrets/passwords before a
spark-submit command is written to task logs or included in exception
messages. The regex it uses to find and mask those values relies on two
unbounded lazy quantifiers (`\S*?`) combined with a negative-lookahead loop
(`(?:(?!\2\s).)*`). On adversarial input this combination causes the regex
engine to explore a quadratic number of backtracking states before it can
conclude there's nothing to mask.
## Root cause
`connection_cmd` includes `self._application_args`, a templated field that
can be populated from the `application_args` key in a DAG run's trigger
`conf`. An authenticated user with DAG-trigger permission can pass an
arbitrarily long string there. If that string doesn't happen to contain
"secret" or "password", the regex still has to *try* to find those literals
at every possible starting offset via `\S*?`, and for each offset it can
expand across the rest of the (whitespace-free) string — O(n) work per
offset, O(n) offsets, O(n²) total. Empirically this hits ~15s at 30,000
characters and grows from there.
Because `_mask_cmd` runs synchronously in the worker process (both when
logging the submit command and when formatting the exception on failure),
a single crafted DAG run can pin a worker slot for the duration of the
regex, and repeated triggers can exhaust the worker pool — a
resource-exhaustion DoS with no code execution or data exposure involved.
## Fix
Two changes to `_mask_cmd`, both in `spark_submit.py`:
1. **Bound the quantifiers.** `\S*?` → `\S{0,100}?` for the key-side
lookaround,
and the unbounded `(?:(?!\2\s).)*` value-consuming loop → bounded to 1024
characters. This caps the amount of backtracking the engine can ever do
per starting position, turning the worst case from quadratic into linear.
The bounds (100 chars for a flag/key name, 1024 chars for a secret value)
are generous for any realistic spark-submit argument while making the
attack surface bounded regardless of how long the attacker-supplied
input is.
2. **Cheap pre-check.** Before running the regex at all, check whether the
(lowercased) command string contains "secret" or "password" as a plain
substring. If neither is present there is nothing to mask, so we return
immediately. This means the overwhelmingly common case (no secrets in
the command at all) never touches the regex, and it means a long
attacker-controlled argument that doesn't contain those words is
rejected in O(n) before any regex work happens.
I deliberately did *not* use the `\S+` (greedy, unbounded) fix suggested in
the original report, because it would break masking of quoted values that
contain embedded spaces (e.g. `--password="my secret value"`), which the
existing test suite already covers. Bounding the quantifiers instead of
changing their semantics keeps masking behavior identical for all realistic
inputs while removing the pathological case.
## Behavior change
None, for any legitimate input. Masking output is byte-for-byte identical
to before for every case in the existing `test_masks_passwords` parametrized
suite. The only behavioral difference is on adversarial input that exceeds
the new bounds (e.g. a "secret" 2000+ characters long with no delimiter) —
in that vanishingly unlikely edge case, masking may stop partway through
the value instead of masking the full length. This is an intentional,
documented trade-off to eliminate the DoS; realistic secrets are nowhere
near 1024 characters.
## Testing
- Ran the full existing `test_masks_passwords` parametrized suite (8 cases)
against the patched regex — all pass with unchanged output.
- Added two regression tests:
- `test_mask_cmd_does_not_hang_on_adversarial_application_args` — asserts
a 200,000-character adversarial payload with no "secret"/"password"
completes in under 2 seconds and leaves the (non-sensitive) command
unmodified.
- `test_mask_cmd_stays_fast_when_trigger_word_present_in_long_input` —
same idea, but with "password=" present so the bounded regex actually
runs instead of hitting the fast-path pre-check.
- Manually reproduced the original ReDoS against the pre-fix regex
(30,000 chars → ~14.7s, consistent with the report's ~2s/50,000 chars
claim adjusted for this environment) and confirmed the fixed regex
resolves the same input in ~0.02s.
- Confirmed linear scaling on the regex-exercising case up to 400,000
characters (roughly 2x time for 2x input, not 4x).
- `ruff check` / `ruff format --check` — no issues introduced on the
changed lines.
closes: #70676
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude (Sonnet 5, Anthropic)
Generated-by: Claude Sonnet 5 (Anthropic) following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
