Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ endif

all: coraza/coraza.h libcoraza.a libcoraza.$(SHARED_EXT)

coraza/coraza.h: libcoraza/coraza.go libcoraza/log.go
coraza/coraza.h: libcoraza/coraza.go libcoraza/log.go libcoraza/fs.go
@mkdir -p coraza
go tool cgo -exportheader $@ $^

libcoraza.a: libcoraza/coraza.go libcoraza/log.go
libcoraza.a: libcoraza/coraza.go libcoraza/log.go libcoraza/fs.go
go build -buildmode=c-archive -o $@ $^

libcoraza.$(SHARED_EXT): libcoraza/coraza.go libcoraza/log.go
libcoraza.$(SHARED_EXT): libcoraza/coraza.go libcoraza/log.go libcoraza/fs.go
go build -buildmode=c-shared -o $@ $^

install-data-local: all check
Expand Down
2 changes: 1 addition & 1 deletion libcoraza/coraza.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ type WafHandle struct {
//export coraza_new_waf_config
func coraza_new_waf_config() C.coraza_waf_config_t {
return C.coraza_waf_config_t(cgo.NewHandle(&WafConfigHandle{
config: coraza.NewWAFConfig(),
config: coraza.NewWAFConfig().WithRootFS(rootFS),
}))
}

Comment on lines +89 to 92
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting a default RootFS (WithRootFS(rootFS)) changes the default security posture: configs/rules that contain Include directives (or other file-reading directives) can now read from the process filesystem. If rule sources can be influenced by untrusted input, this becomes a local file read primitive. Consider making filesystem access opt-in (expose a C API to set RootFS / allowed include dirs) or at least document the new behavior and restrict the default FS scope (e.g., only a dedicated rules directory).

Suggested change
config: coraza.NewWAFConfig().WithRootFS(rootFS),
}))
}
config: coraza.NewWAFConfig(),
}))
}
//export coraza_waf_config_set_rootfs
func coraza_waf_config_set_rootfs(c C.coraza_waf_config_t, rootPath *C.char) C.int {
configHandle := fromRaw[*WafConfigHandle](c)
if rootPath == nil {
return 1
}
path := C.GoString(rootPath)
if path == "" {
return 1
}
configHandle.config = configHandle.config.WithRootFS(os.DirFS(path))
return 0
}

Copilot uses AI. Check for mistakes.
Expand Down
82 changes: 82 additions & 0 deletions libcoraza/coraza_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,88 @@ func TestWafIsConsistent(t *testing.T) {
}

func TestAddRulesToWaf(t *testing.T) {
tests := []struct {
name string
rules string
canCreateWaf bool
}{
{
name: "rule",
rules: `SecRule REMOTE_ADDR "127.0.0.1" "id:1,phase:1,deny,log,msg:'test 123',status:403"`,
canCreateWaf: true,
},
{
name: "include local file",
rules: `Include testdata/test.conf`,
canCreateWaf: true,
},
{
name: "include invalid rule",
rules: `foobar123`,
canCreateWaf: false,
},
{
name: "include non-existent file",
rules: `Include testdata/nonexistent.conf`,
canCreateWaf: false,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
config := coraza_new_waf_config()
rv := coraza_rules_add(config, stringToC(test.rules))
if rv != 0 {
t.Fatalf("Rules addition failed: %d", rv)
}

er := stringToC("")
waf := coraza_new_waf(config, &er)
if test.canCreateWaf && (waf == 0 || stringFromC(er) != "") {
t.Fatalf("Waf creation failed: %d", waf)
} else if !test.canCreateWaf && (waf != 0 || stringFromC(er) == "") {
t.Fatalf("Waf creation should have failed: %d", waf)
}
Comment on lines +64 to +80
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These subtests create WAF configs/WAF instances but never free them. Other tests in this file consistently call coraza_free_waf_config / coraza_free_waf, so leaving handles allocated here can inflate memory usage across the full test suite and hide leaks. Consider deferring the appropriate free calls inside each subtest after successful creation.

Copilot uses AI. Check for mistakes.
if stringFromC(er) != "" {
t.Logf("Waf creation error: %s", stringFromC(er))
}
})
}
}

func TestAddRulesFromFileToWaf(t *testing.T) {
tests := []struct {
name string
file string
canCreateWaf bool
}{
{
name: "test.conf",
file: "testdata/test.conf",
canCreateWaf: true,
},
{
name: "nonexistent.conf",
file: "testdata/nonexistent.conf",
canCreateWaf: false,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
config := coraza_new_waf_config()
rv := coraza_rules_add_file(config, stringToC(test.file))
if rv != 0 {
t.Fatalf("Rules addition failed: %d", rv)
}

er := stringToC("")
waf := coraza_new_waf(config, &er)
if test.canCreateWaf && (waf == 0 || stringFromC(er) != "") {
t.Fatalf("Waf creation failed: %d", waf)
Comment on lines +103 to +116
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestAddRulesFromFileToWaf also creates configs/WAFs without freeing them. To keep tests consistent and avoid leaking cgo handles across the suite, add coraza_free_waf_config / coraza_free_waf cleanup (and any error-string freeing if applicable) within each subtest.

Copilot uses AI. Check for mistakes.
} else if !test.canCreateWaf && (waf != 0 || stringFromC(er) == "") {
t.Fatalf("Waf creation should have failed: %d", waf)
}
})
}
}

func TestRulesCount(t *testing.T) {
Expand Down
32 changes: 32 additions & 0 deletions libcoraza/fs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package main

import "C"

import (
"io/fs"
"os"
"path"
)

// combinedFS is a filesystem that routes paths by checking both the local and root filesystems.
type combinedFS struct {
localFS fs.FS // local filesystem
rootFS fs.FS // root filesystem
}

func (c *combinedFS) Open(name string) (fs.File, error) {
if path.IsAbs(name) {
return c.rootFS.Open(name)
} else {
return c.localFS.Open(name)
}
Comment on lines +17 to +22
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.DirFS("/") expects paths without a leading slash (it enforces fs.ValidPath). When name is absolute (e.g. "/etc/crs.conf"), c.rootFS.Open(name) will fail with an invalid path error, so absolute-path Includes won't work. Consider trimming the leading "/" (and rejecting "/" itself) before calling rootFS.Open, and ensure any cleaned path still passes fs.ValidPath to prevent traversal.

Copilot uses AI. Check for mistakes.
}

var rootFS fs.FS

func init() {
rootFS = &combinedFS{
localFS: os.DirFS("."),
rootFS: os.DirFS("/"),
}
}
1 change: 1 addition & 0 deletions libcoraza/testdata/test.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
SecRule REMOTE_ADDR "127.0.0.1" "id:1,phase:1,deny,log,msg:'test 123',status:403"
Loading