Path rules and trap paths match the path decoded once too (closes #85)
check / check (push) Waiting to run
check / check (push) Waiting to run
A path rule matched only the path as the client sent it, so /%2eenv got past the default env-file rule while an app that decodes the path serves /.env. A path rule now matches the path as sent or decoded once, as a uri rule already did, and so does a path in SWWAF_TRAP_PATHS: /%77p-login.php matches /wp-login.php. The query is cut off before the path is decoded, and the path is decoded only once, so /%252eenv is not /.env. A ban's notes keep the path as sent. SPEC.md and README.md say so. Model: opus-5-5
This commit is contained in:
@@ -10,13 +10,15 @@ import (
|
||||
)
|
||||
|
||||
// trapPath reports whether the request asks for a path in
|
||||
// SWWAF_TRAP_PATHS: its path as a path rule sees it, before any decoding
|
||||
// and without the query, is one of them. Such a request is a clear sign of
|
||||
// attack, as a ban rule's match is: it bans the client's netblock, or in
|
||||
// observe mode raises the alert for the ban it would have made.
|
||||
// SWWAF_TRAP_PATHS: its path as a path rule sees it, without the query,
|
||||
// as sent or once percent-decoded, is one of them. Such a request is a
|
||||
// clear sign of attack, as a ban rule's match is: it bans the client's
|
||||
// netblock, or in observe mode raises the alert for the ban it would have
|
||||
// made. The ban's notes give the path as sent.
|
||||
func (rq *request) trapPath(now time.Time) bool {
|
||||
path := rules.Path(rq.in)
|
||||
if !slices.Contains(rq.h.config.TrapPaths, path) {
|
||||
if !slices.Contains(rq.h.config.TrapPaths, path) &&
|
||||
!slices.Contains(rq.h.config.TrapPaths, rules.DecodeOnce(path)) {
|
||||
return false
|
||||
}
|
||||
|
||||
|
||||
@@ -32,10 +32,9 @@ func TestTrapPathBansAsABanRuleDoes(t *testing.T) {
|
||||
})
|
||||
start := clk.Now()
|
||||
|
||||
// Only the path itself, as the client sent it, is a trap path.
|
||||
for _, path := range []string{
|
||||
"/wp-login.php/", "/WP-LOGIN.PHP", "/blog/xmlrpc.php", "/%77p-login.php",
|
||||
} {
|
||||
// Only the path itself, the whole of it in the same case, is a trap
|
||||
// path.
|
||||
for _, path := range []string{"/wp-login.php/", "/WP-LOGIN.PHP", "/blog/xmlrpc.php"} {
|
||||
s.request(otherClient, path, http.StatusOK, requestlog.ActionForward)
|
||||
}
|
||||
|
||||
@@ -84,6 +83,22 @@ func TestTrapPathBansAsABanRuleDoes(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestTrapPathMatchesThePathDecodedOnce(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s, _, server := startWithClock(t, "", map[string]string{trapPaths: trapPathList})
|
||||
|
||||
// %2577 decodes once to %77, not to w.
|
||||
s.request(otherClient, "/%2577p-login.php", http.StatusOK, requestlog.ActionForward)
|
||||
s.request(client, "/%77p-login.php", http.StatusForbidden, requestlog.ActionBanned)
|
||||
|
||||
// The ban's notes give the path as the client sent it.
|
||||
got := server.Ledger.Bans(netip.MustParsePrefix(client + "/32"))
|
||||
if len(got) != 1 || got[0].Notes.TrapPath != "/%77p-login.php" {
|
||||
t.Fatalf("bans %+v, want one whose notes give the trap path /%%77p-login.php", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTrapPathsNeedNoRuleFiles(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user