[airween]

Proposed changes

Document that SESSION has to be initialized with setsid before it is used, as agreed in owasp-modsecurity/ModSecurity#2442.

• Variables, SESSION: the entry said that SESSION "becomes available only after setsid is executed". That isn't what libModSecurity v3 does, so it now says that SESSION must be initialized with setsid, and a warning describes the v3 behavior:
  • without setsid, the data is stored without a session key, so it is shared by every transaction that does the same, whatever session it belongs to;
  • in an LMDB build, such a variable can also overwrite, and be overwritten by, a variable with the same name in another collection that has no key either, for example GLOBAL used without initcol. This is what the issue reported.
• Actions, setsid: one sentence in the existing note, with a link to the SESSION warning.

Closes owasp-modsecurity/ModSecurity#2442

Further comments

The LMDB part is based on the current v3 source: the LMDB backend keeps all collections in one unnamed database (src/collection/backend/lmdb.cc), the key is compartment::webappid::name without the collection name (headers/modsecurity/collection/collection.h), and only setsid sets the SESSION key (src/actions/set_sid.cc). With the default in-memory backend, SESSION is a separate object, so it doesn't collide with other collections, but the data is still shared by all sessions.

🤖 Generated with help of Claude Code, Opus 5.5

[fzipi]

Thanks @airween. I checked the v3 claims against v3/master, and they hold:

• SetSID::evaluate() is the only place that sets m_session_collection_key, which is empty by default, so SESSION without setsid is read and written under an empty key.
• With LMDB, every collection gets its env and DBI from the MDBEnvProvider singleton, which opens one unnamed database (mdb_dbi_open(txn, NULL, ...)). The stored key is compartment::webappid::name, with no collection name, so SESSION and GLOBAL without a key can collide.
• With the in-memory backend, SESSION is its own InMemoryPerProcess object, as the description says.

One problem: the example under the warning does what the warning says not to do. The warning says to use SESSION "only after setsid has run with a non-empty session token, for example by checking the token in the rule that calls setsid, as in the example below". But checking the token in rule 70 only skips the setsid. Rules 71 and 72 still run without a session. I ran the example on v3/master with a request to /cgi-bin/finger and no cookie:

(Rule: 70) Executing operator "Rx" with param "^$" against REQUEST_COOKIES:PHPSESSID.
(Rule: 71) Executing operator "Rx" with param "^/cgi-bin/finger$" against REQUEST_URI.
Saving variable: SESSION:score with value: 10

So every request without the cookie adds to the same shared score, which is the case the warning describes. Without setsid, SESSIONID is an empty string (I checked: SecRule SESSIONID "@rx ^$" matches), so the example could skip the SESSION rules when there is no session:

SecRule REQUEST_COOKIES:PHPSESSID "!@rx ^$" \
    "id:70,phase:2,pass,nolog,setsid:%{REQUEST_COOKIES.PHPSESSID}"

# Skip the SESSION rules if setsid didn't run
SecRule SESSIONID "@rx ^$" \
    "id:73,phase:2,pass,nolog,skipAfter:END_SESSION_RULES"

(rules 71, 72 and the deny rule)

SecMarker END_SESSION_RULES

Then the warning could say "for example by skipping the rules that use SESSION when SESSIONID is empty, as in the example below". I only tested this on v3. In v2, SESSION without setsid isn't available at all, so the guard is harmless there.

[airween]

Thanks @fzipi, you're right: checking the token in rule 70 only skips setsid, not the rules that use SESSION.

I added your SESSIONID guard with skipAfter:END_SESSION_RULES, and changed the warning as you suggested. Your guard couldn't keep id:73, because the deny rule already uses it, so the guard is now id:71, and the rules after it are 72–74.