v3: setvar added by SecRuleUpdateActionById to a chain starter only runs on full chain match #3707

Describe the bug

In libModSecurity v3, a setvar written directly on a chain starter runs as soon as the starter matches, even when a later rule in the chain fails. That matches v2 and the documented behavior of chain.

A setvar added to the same rule with SecRuleUpdateActionById behaves differently: it only runs when the whole chain matches. As a result, updating a rule's actions changes when its variables are set, depending only on how the setvar was attached.

To Reproduce

Request headers: User-Agent: curl/7.38.0, Accept: */*.

SecRuleEngine On
SecAction "id:1,phase:1,pass,nolog,setvar:tx.a=0"
SecRule REQUEST_HEADERS:User-Agent "@rx ^curl" "id:10,phase:2,pass,log,setvar:tx.a=+1,chain"
    SecRule REQUEST_HEADERS:Accept "@rx nomatch" "setvar:tx.a=+2"
SecRuleUpdateActionById 10 "setvar:tx.a=+8"
SecRule TX:a "@eq 9" "id:999,phase:2,deny,status:403,log"

Expected behavior

The starter (id 10) matches and the chained rule fails, so both setvar actions on rule 10 should run: the inline one (+1) and the updated one (+8). Result tx.a == 9, and rule 999 denies with 403.

Actual behavior

tx.a == 1: only the inline setvar ran. If the chained rule is changed to match (@rx \*/\*), tx.a == 11 (1 + 2 + 8). So the updated setvar does run, but only on a full chain match.

Measured with test/regression_tests on v3/master (84b5996).

Cause

RulesExceptions::loadUpdateActionById (src/rules_exceptions.cc) sorts update actions by action_kind. SetVar uses the default RunTimeOnlyIfMatchKind, so it goes into m_action_pos_update_target_by_id. Those actions run in RuleWithActions::executeActionsAfterFullMatch, which only runs once the whole chain has matched.

RuleWithActions::executeActionsIndependentOfChainedRuleResult (src/rule_with_actions.cc, around line 212) has a branch meant to run an updated setvar early. It only iterates m_action_pre_update_target_by_id, though, and a setvar never lands in that list, so the branch is dead for setvar.

The fix is probably to run updated setvar actions from the pos list in the "independent" path, and skip them in executeActionsAfterFullMatch. That would mirror how inline m_actionsSetVar is handled.

Context

Found while documenting chain behavior for #2860 (owasp-modsecurity/documentation#2). The docs currently describe the v3 behavior as it is. If this gets fixed, the "Updating chained rules" note under SecRuleUpdateActionById will need updating.