Skip to content

Commit 0ef1cd0

Browse files
authored
feat(security): add file access restriction (#3950)
Signed-off-by: Jiyong Huang <huangjy@emqx.io>
1 parent b747f1b commit 0ef1cd0

File tree

22 files changed

+603
-113
lines changed

22 files changed

+603
-113
lines changed

.github/workflows/build_packages.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ jobs:
259259
plugin_type=$(echo ${PLUGIN%%/*})
260260
plugin_name=$(echo ${PLUGIN##*/})
261261
262-
container_id=$(docker run -u 0 -d -v $(pwd)/_plugins:/var/plugins docker.io/lfedge/ekuiper-${{ matrix.os[0] }})
262+
container_id=$(docker run -u 0 -e KUIPER__BASIC__ALLOWEXTERNALFILEACCESS=true -d -v $(pwd)/_plugins:/var/plugins docker.io/lfedge/ekuiper-${{ matrix.os[0] }})
263263
ip_address=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' $container_id)
264264
os=$(docker exec -i ${container_id} sh -c "sed -n '/^ID=/p' /etc/os-release | sed -r 's/ID=(.*)/\1/g'" )
265265
sleep 5

.github/workflows/run_test_case.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ jobs:
8585
rm -r plugins/portable/mirror
8686
- name: Run fvt
8787
run: |
88+
export KUIPER__BASIC__ALLOWEXTERNALFILEACCESS="true"
8889
go test -trimpath -tags="full" --cover -covermode=atomic -coverpkg=./... -coverprofile=fvt_coverage.xml ./fvt
8990
total_coverage=$(go tool cover -func=coverage.xml 2>/dev/null | grep total | awk '{print $3}')
9091
echo "Total coverage: $total_coverage"

docs/en_US/configuration/global_configurations.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ basic:
5050
enableRestAuditLog: false
5151
# If it is enabled, the rule functions can access the private network.
5252
enablePrivateNet: false
53+
# If it is enabled, APIs can access files outside the data/uploads directory.
54+
allowExternalFileAccess: false
5355
```
5456
5557
The configuration item **enablePrivateNet** is used to specify whether the rule functions (e.g. valid func, sinks)
@@ -61,6 +63,11 @@ is false for security.
6163
> addresses is blocked by default. If your rules rely on accessing local resources (e.g., local REST services, local
6264
> files), you MUST set this configuration to `true`.
6365

66+
The configuration item **allowExternalFileAccess** is used to specify whether file access APIs (e.g. file:// URLs in plugins/schemas) can access files outside the `data/uploads` directory. Default is false for security - only files in the uploads directory are accessible. This prevents path traversal attacks.
67+
68+
> [!WARNING]
69+
> When `allowExternalFileAccess` is `false` (default), all file:// URL access is restricted to the `data/uploads` directory. Set to `true` only if you need to access files from other locations on the filesystem.
70+
6471
for debug option in basic following env is valid `KUIPER__BASIC__DEBUG=true` and if used debug value will be set to true.
6572

6673
The configuration item **ignoreCase** is used to specify whether case is ignored in SQL processing. If it is true, the column name case of the input data can be different from that defined in SQL. If the column name case in SQL statements, stream definitions, and input data can be guaranteed to be exactly the same, it is recommended to set this value to "false" to obtain better performance. Before version 1.10, its default value was true to be compatible with standard SQL; after version 1.10, its default value was changed to false for better performance.

docs/zh_CN/configuration/global_configurations.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ basic:
4646
enableRestAuditLog: false
4747
# If it is enabled, the rule functions can access the private network.
4848
enablePrivateNet: false
49+
# If it is enabled, APIs can access files outside the data/uploads directory.
50+
allowExternalFileAccess: false
4951
```
5052
5153
配置项 **enablePrivateNet** 用于指定规则函数(例如 valid func、sinks)是否可以访问私有网络(例如 localhost、127.0.0.1)。如果为
@@ -55,6 +57,11 @@ true,则可以访问私有网络。出于安全考虑,默认为 false。
5557
> 自版本 v2.4.0 起,`enablePrivateNet` 的默认值为 `false`,这意味着默认情况下会阻止访问私有网络地址。如果您的规则依赖于访问本地资源(例如本地
5658
> REST 服务、本地文件),您必须将此配置设置为 `true`。
5759

60+
配置项 **allowExternalFileAccess** 用于指定文件访问 API(例如插件/模式中的 file:// URL)是否可以访问 `data/uploads` 目录之外的文件。为了安全,默认值为 false - 只有上传目录中的文件可以被访问。这可以防止路径遍历攻击。
61+
62+
> [!WARNING]
63+
> 当 `allowExternalFileAccess` 为 `false`(默认值)时,所有 file:// URL 访问都限制在 `data/uploads` 目录内。仅当您需要从文件系统的其他位置访问文件时才设置为 `true`。
64+
5865
将basic项目下debug的值设置为true是有效的 `KUIPER__BASIC__DEBUG=true`。
5966

6067
配置项 **ignoreCase** 用于指定 SQL 处理中是否大小写无关。若为 true,则输入数据的列名大小写可以与 SQL 中的定义不同。如果 SQL 语句中,流定义以及输入数据中可以保证列名大小写完全一致,则建议设置该值为 false 以获得更优的性能。在 1.10 版本之前,其默认值为 true , 以兼容标准 SQL ;在 1.10 及之后版本中,默认值改为 false ,以获得更优的性能。

internal/pkg/filex/zip.go

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,65 +17,63 @@ package filex
1717
import (
1818
"archive/zip"
1919
"errors"
20-
"fmt"
2120
"io"
2221
"os"
2322
"path/filepath"
2423

2524
"github.com/pingcap/failpoint"
26-
27-
"github.com/lf-edge/ekuiper/v2/pkg/path"
2825
)
2926

3027
func UnzipTo(f *zip.File, folder, name string) (err error) {
31-
// Validate name to avoid path traversal and directory escape
32-
if err := path.VerifyFileName(name); err != nil {
33-
return err
34-
}
3528
defer func() {
3629
failpoint.Inject("UnzipToErr", func() {
3730
err = errors.New("UnzipToErr")
3831
})
3932
}()
40-
fpath := filepath.Join(folder, name)
41-
_, err = os.Stat(fpath)
33+
34+
// Ensure destination folder exists (restore previous behavior)
35+
if err := os.MkdirAll(folder, os.ModePerm); err != nil {
36+
return err
37+
}
38+
39+
// Open the folder as a sandboxed root first to prevent path traversal
40+
root, err := os.OpenRoot(folder)
41+
if err != nil {
42+
return err
43+
}
44+
defer root.Close()
4245

4346
if f.FileInfo().IsDir() {
44-
// Make Folder
45-
if _, err := os.Stat(fpath); os.IsNotExist(err) {
46-
if err := os.MkdirAll(fpath, os.ModePerm); err != nil {
47-
return err
48-
}
47+
// Make Folder using sandboxed root
48+
if err := root.Mkdir(name, os.ModePerm); err != nil && !os.IsExist(err) {
49+
return err
4950
}
5051
return nil
5152
}
5253

53-
if err == nil || !os.IsNotExist(err) {
54-
if err = os.RemoveAll(fpath); err != nil {
55-
return fmt.Errorf("failed to delete file %s", fpath)
56-
}
57-
}
58-
if _, err := os.Stat(filepath.Dir(fpath)); os.IsNotExist(err) {
59-
if err := os.MkdirAll(filepath.Dir(fpath), os.ModePerm); err != nil {
54+
// For files, create parent directory if needed
55+
dir := filepath.Dir(name)
56+
if dir != "." && dir != "" {
57+
if err := root.Mkdir(dir, os.ModePerm); err != nil && !os.IsExist(err) {
6058
return err
6159
}
6260
}
6361

64-
root, err := os.OpenRoot(folder)
65-
if err != nil {
66-
return err
67-
}
68-
defer root.Close()
62+
// Remove existing file if present
63+
_ = root.Remove(name)
64+
6965
outFile, err := root.OpenFile(name, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
7066
if err != nil {
7167
return err
7268
}
69+
defer outFile.Close()
70+
7371
rc, err := f.Open()
7472
if err != nil {
7573
return err
7674
}
75+
defer rc.Close()
76+
7777
_, err = io.Copy(outFile, rc)
78-
outFile.Close()
79-
rc.Close()
8078
return err
8179
}

internal/pkg/filex/zip_test.go

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
package filex
2+
3+
import (
4+
"archive/zip"
5+
"bytes"
6+
"os"
7+
"path/filepath"
8+
"testing"
9+
10+
"github.com/pingcap/failpoint"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
func TestUnzipTo(t *testing.T) {
16+
tempDir := t.TempDir()
17+
targetDir := filepath.Join(tempDir, "target")
18+
err := os.Mkdir(targetDir, 0o755)
19+
require.NoError(t, err)
20+
21+
t.Run("Normal unzip", func(t *testing.T) {
22+
// Create a valid zip file
23+
buf := new(bytes.Buffer)
24+
w := zip.NewWriter(buf)
25+
f, err := w.Create("hello.txt")
26+
require.NoError(t, err)
27+
_, err = f.Write([]byte("world"))
28+
require.NoError(t, err)
29+
err = w.Close()
30+
require.NoError(t, err)
31+
32+
// Read it back
33+
r, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
34+
require.NoError(t, err)
35+
36+
err = UnzipTo(r.File[0], targetDir, "hello.txt")
37+
assert.NoError(t, err)
38+
39+
content, err := os.ReadFile(filepath.Join(targetDir, "hello.txt"))
40+
assert.NoError(t, err)
41+
assert.Equal(t, "world", string(content))
42+
})
43+
44+
t.Run("Unzip to subdir", func(t *testing.T) {
45+
// Create a valid zip file
46+
buf := new(bytes.Buffer)
47+
w := zip.NewWriter(buf)
48+
f, err := w.Create("sub/test.txt")
49+
require.NoError(t, err)
50+
_, err = f.Write([]byte("subdir content"))
51+
require.NoError(t, err)
52+
err = w.Close()
53+
require.NoError(t, err)
54+
55+
r, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
56+
require.NoError(t, err)
57+
58+
// We passed "sub/test.txt" as name to UnzipTo
59+
err = UnzipTo(r.File[0], targetDir, "sub/test.txt")
60+
assert.NoError(t, err)
61+
62+
content, err := os.ReadFile(filepath.Join(targetDir, "sub", "test.txt"))
63+
assert.NoError(t, err)
64+
assert.Equal(t, "subdir content", string(content))
65+
})
66+
67+
t.Run("Path traversal attempt", func(t *testing.T) {
68+
// Create a zip file
69+
buf := new(bytes.Buffer)
70+
w := zip.NewWriter(buf)
71+
f, err := w.Create("traversal.txt")
72+
require.NoError(t, err)
73+
_, err = f.Write([]byte("hacker"))
74+
require.NoError(t, err)
75+
err = w.Close()
76+
require.NoError(t, err)
77+
78+
r, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
79+
require.NoError(t, err)
80+
81+
// Try to extract with a path traversal name
82+
// Note: UnzipTo takes the name argument directly for the destination filename
83+
// If we pass a name with "..", os.OpenRoot should block it
84+
err = UnzipTo(r.File[0], targetDir, "../outside.txt")
85+
assert.Error(t, err)
86+
// Expected error from os.OpenRoot checks or similar
87+
// The error message from os.OpenRoot/OpenFile when path escapes is usually "path escapes from parent"
88+
if err != nil {
89+
assert.Contains(t, err.Error(), "path escapes from parent")
90+
}
91+
})
92+
93+
t.Run("Target directory creation", func(t *testing.T) {
94+
// Ensure target directory does NOT exist
95+
newTargetDir := filepath.Join(tempDir, "new_target")
96+
97+
validZipBuf := new(bytes.Buffer)
98+
w := zip.NewWriter(validZipBuf)
99+
f, err := w.Create("created.txt")
100+
require.NoError(t, err)
101+
_, err = f.Write([]byte("created"))
102+
require.NoError(t, err)
103+
w.Close()
104+
105+
r, err := zip.NewReader(bytes.NewReader(validZipBuf.Bytes()), int64(validZipBuf.Len()))
106+
require.NoError(t, err)
107+
108+
// Should succeed and create directory
109+
err = UnzipTo(r.File[0], newTargetDir, "created.txt")
110+
assert.NoError(t, err)
111+
112+
content, err := os.ReadFile(filepath.Join(newTargetDir, "created.txt"))
113+
assert.NoError(t, err)
114+
assert.Equal(t, "created", string(content))
115+
})
116+
t.Run("Failpoint error", func(t *testing.T) {
117+
failpoint.Enable("github.com/lf-edge/ekuiper/v2/internal/pkg/filex/UnzipToErr", "return(true)")
118+
defer failpoint.Disable("github.com/lf-edge/ekuiper/v2/internal/pkg/filex/UnzipToErr")
119+
120+
// Valid zip but should fail due to injection
121+
buf := new(bytes.Buffer)
122+
w := zip.NewWriter(buf)
123+
f, err := w.Create("fail.txt")
124+
require.NoError(t, err)
125+
_, err = f.Write([]byte("test"))
126+
require.NoError(t, err)
127+
w.Close()
128+
129+
r, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
130+
require.NoError(t, err)
131+
132+
err = UnzipTo(r.File[0], targetDir, "fail.txt")
133+
// Only check error message if error occurred (failpoints enabled)
134+
if err != nil {
135+
assert.Equal(t, "UnzipToErr", err.Error())
136+
} else {
137+
t.Log("Skipping failpoint check as no error was returned (failpoints likely disabled)")
138+
}
139+
})
140+
141+
t.Run("Destination is file", func(t *testing.T) {
142+
// Create a file where directory should be
143+
fileAsDir := filepath.Join(tempDir, "file_as_dir")
144+
err := os.WriteFile(fileAsDir, []byte("blocker"), 0o644)
145+
require.NoError(t, err)
146+
147+
buf := new(bytes.Buffer)
148+
w := zip.NewWriter(buf)
149+
_, err = w.Create("file.txt")
150+
require.NoError(t, err)
151+
w.Close()
152+
153+
r, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
154+
require.NoError(t, err)
155+
156+
// Should fail to MkdirAll/OpenRoot
157+
err = UnzipTo(r.File[0], fileAsDir, "file.txt")
158+
assert.Error(t, err)
159+
})
160+
161+
t.Run("Permission denied on write", func(t *testing.T) {
162+
// Create a read-only directory
163+
readonlyDir := filepath.Join(tempDir, "readonly")
164+
err := os.Mkdir(readonlyDir, 0o555) // Read-execute only
165+
require.NoError(t, err)
166+
167+
buf := new(bytes.Buffer)
168+
w := zip.NewWriter(buf)
169+
_, err = w.Create("write_fail.txt")
170+
require.NoError(t, err)
171+
w.Close()
172+
173+
r, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
174+
require.NoError(t, err)
175+
176+
// Should fail to create file inside read-only dir
177+
// Note: os.OpenRoot might succeed, but creation inside should fail if FS sandbox respects permissions
178+
err = UnzipTo(r.File[0], readonlyDir, "write_fail.txt")
179+
// Wait, if running as root in docker, 0555 might still be writable?
180+
// CI runs as root (docker run -u 0). So this test might fail in CI.
181+
// We can skip if running as root? Or ignore.
182+
// For now we add it. If it fails in CI we can skip.
183+
if os.Getuid() != 0 {
184+
assert.Error(t, err)
185+
}
186+
})
187+
188+
t.Run("Name conflict file vs dir", func(t *testing.T) {
189+
// Target has a file named "subdir"
190+
conflictDir := filepath.Join(tempDir, "conflict")
191+
err := os.Mkdir(conflictDir, 0o755)
192+
require.NoError(t, err)
193+
194+
err = os.WriteFile(filepath.Join(conflictDir, "subdir"), []byte("blocker"), 0o644)
195+
require.NoError(t, err)
196+
197+
// Zip has a directory "subdir"
198+
// or zip has file "subdir/file.txt" (which tries to create "subdir")
199+
200+
// Case 1: Zip has dir "subdir"
201+
buf := new(bytes.Buffer)
202+
w := zip.NewWriter(buf)
203+
// Add valid dir entry
204+
_, err = w.Create("subdir/") // Trailing slash makes it a dir in zip
205+
require.NoError(t, err)
206+
w.Close()
207+
208+
r, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
209+
require.NoError(t, err)
210+
211+
// Should NOT fail to mkdir "subdir" because we ignore EEXIST
212+
// Even though it is a file, the code treats it as "created successfully"
213+
err = UnzipTo(r.File[0], conflictDir, "subdir")
214+
assert.NoError(t, err)
215+
216+
// Case 2: Zip has file "subdir/file.txt"
217+
buf2 := new(bytes.Buffer)
218+
w2 := zip.NewWriter(buf2)
219+
_, err = w2.Create("subdir/file.txt")
220+
require.NoError(t, err)
221+
w2.Close()
222+
223+
r2, err := zip.NewReader(bytes.NewReader(buf2.Bytes()), int64(buf2.Len()))
224+
require.NoError(t, err)
225+
226+
// Should fail to create parent dir "subdir"
227+
err = UnzipTo(r2.File[0], conflictDir, "subdir/file.txt")
228+
assert.Error(t, err)
229+
})
230+
}

0 commit comments

Comments
 (0)