bryancall commented on PR #13684:
URL: https://github.com/apache/trafficserver/pull/13684#issuecomment-5683350847

   Fixed in 473b8ddc2e. The Copilot finding was filed as a suppressed comment 
with no thread, so this is the reply.
   
   I verified the claim in the source rather than taking it on trust, and it 
holds exactly as described. `RegexMatcher::NewEntry` discarded what `compile()` 
returned and used `empty()` as the compile-status check 
(`src/proxy/ControlMatcher.cc:428`), and the slot really is reused: when a 
line's `Data::Init()` fails, the function resets `regex_strings[num_el]` but 
leaves `num_el` alone, so the next line lands on the same `Regex`, still 
holding the pattern the rejected line compiled.
   
   Worth being precise about what changed, because the caller was already 
broken. Before this PR a failed `compile()` left a *freed* pointer in `_code`, 
so `empty()` answered false there too and the bad line was accepted, then 
matched through freed memory. This PR does not introduce the bug; it changes 
the symptom from a use after free to a line that is accepted and matched 
against the previous pattern under its own configuration. Quieter, which is a 
good reason to fix the caller in the same change rather than leave it.
   
   `NewEntry` now asks `compile()` whether it compiled.
   
   I also checked the rest of the tree for the same shape. Five call sites 
discard the `compile()` result: `ControlMatcher.cc:428`, `SSLSNIConfig.cc:103`, 
`Diags.cc:375`, `Regression.cc:97`, and `regex_revalidate.cc:215`. This change 
is only observable when a `Regex` that already holds a pattern is recompiled 
and the new pattern fails, and those other four compile into a freshly 
constructed object, where a failed compile leaves it empty exactly as before. 
`ControlMatcher` is the one with a reachable reuse path, so it is the one that 
needed fixing.
   
   `ctest -R proxy` passes 86/86 with the change and 86/86 on unmodified 
master, so the existing `test_ControlMatcher` coverage is unaffected.
   


-- 
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]

Reply via email to