fix(security): enforce base_path boundary in getActualPath and add S3 gateway authorization - #82
Open
Mcchen1008 wants to merge 1 commit into
Open
Mcchen1008 wants to merge 1 commit into
Mcchen1008 wants to merge 1 commit into
Conversation
… gateway authorization
Two P0 authorization bypasses:
1. base_path jail escape via dot segments (pkg/permission.ts)
getActualPath() only string-concatenated the user base_path with the
request path. resolvePath() then folds ".." with a stack clamped to
the virtual root "/", popping the base_path prefix back off. A user
with base_path=/jail could therefore reach ANY mounted storage:
POST /api/fs/get {"path":"/../secret"} -> resolves to /secret
POST /api/fs/put {"path":"/../x/..."} -> cross-storage write
Encoded variants (%2e%2e, %2f splitting, %5c, "....") also bypassed
the fold because decoding happened only inside resolvePath, after
concatenation.
Fix: getActualPath() now normalizes + collapses the joined path with
rules kept in lockstep with resolvePath (decode-until-stable,
separator/dot-run folding) and clamps any escape back to the user
root. Applied at the single choke point used by every fs/raw/share
endpoint, plus seed paths.
2. /s3/* gateway ignored permissions entirely (server/s3.ts, auth.ts)
authUserFromReq() did not reject disabled users (unlike
getUserFromContext), and the S3 gateway only checked "a valid JWT
exists": no permission bits, no base_path confinement. Any user
(including disabled ones) could read/write/delete the whole virtual
filesystem via /s3/<mount>/<key>.
Fix:
- authUserFromReq() now rejects disabled users
- S3 reads/list are confined through getActualPath()
- PutObject requires WRITE_CONTENT, DeleteObject requires DELETE
- guests are rejected defensively
- malformed percent-encoding no longer 500s (safe decode fallback)
- GET /s3/ (trailing slash, what S3 clients actually send) now
reaches ListBuckets instead of falling through to the /* GetObject
route and returning NoSuchKey
Tests: 16 new regression tests (pkg/permission.test.ts,
server/s3_auth.test.ts) covering plain/encoded traversal clamping,
disabled-user rejection, permission-bit gates, base_path confinement
for list/get, and admin non-regression. Full suite shows zero
regressions (server 130/135, model 39/39, store 12/12 — the 5+1
pre-existing failures are identical on main).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概述
本 PR 修复代码审查中发现的 2 个 P0 级授权绕过漏洞,并附带 16 个回归测试。
P0-1:
base_path监禁可被..段完全穿越(跨存储越权读/写/删)根因:
getActualPath()(src/backend/pkg/permission.ts)此前仅做字符串拼接,而下游resolvePath()的..折叠只钳制到虚拟根/——拼接上去的base_path前缀会被..直接"弹掉":实测影响(低权限、无特权位、base_path=/jail 的用户):
POST /api/fs/get {"path":"/../secret"}→ 200,读取其他存储内容POST /api/fs/put同理可越权写入;list/move/copy/remove/rename 全部同根因%2e%2e(单编码)、%252e%252e(双编码)、sub%2f..%2f..(编码斜杠分裂)、%5c、....(点串)——因为解码只发生在resolvePath内部(拼接之后)修复:
getActualPath()现在把拼接后的路径按与resolvePath完全同步的规则(解码至稳定 + 分隔符/点串规范化 + 栈折叠)收敛,逃出base_path一律钳制回用户根目录。由于 fs/raw/share 全部端点都经由getActualPath这一单点,一处修复覆盖全部入口;base_path自身含..的错误配置也会被折叠。P0-2:
/s3/*网关完全不校验权限(任意用户读写删全盘,含禁用用户)根因(两个缺陷叠加):
authUserFromReq()(src/backend/server/auth.ts)不拒绝disabled用户(与getUserFromContext不一致)——被禁用用户的存量 JWT 仍可通过/s3、/dav(Bearer) 等入口;server/s3.ts只检查"存在有效 JWT":不查权限位、不经getActualPath()收敛路径。permission=0、base_path=/jail的普通用户即可GET/PUT/DELETE /s3/<任意挂载>/<key>读写删整个虚拟文件系统。修复:
authUserFromReq()补user.disabled拒绝;getActualPath()收敛到base_path;WRITE_CONTENT、DeleteObject 要求DELETE权限位(与/api/fs/put、/api/fs/remove对齐);附带修复
GET /s3/(带尾斜杠——S3 客户端实际发送的形式)此前落入/*GetObject 路由返回 NoSuchKey,ListBuckets 仅GET /s3可达;现抽出共用 handler,两种形式均返回 bucket 列表;parseS3Path中畸形百分号编码不再导致 500(安全解码回退)。测试
新增 16 个回归测试,全部通过:
src/backend/pkg/permission.test.ts(10 项):明文/编码穿越钳制、监禁内相对跳转保留、反斜杠与多斜杠规范化、base_path自身折叠、guest/disabled/admin 权限语义;src/backend/server/s3_auth.test.ts(6 项):禁用用户 403、无权限位 PUT/DELETE 403、base_path 收敛 ListBuckets/GetObject、..不逃逸且不泄露上游 302、admin 行为不变。全量套件零回归:
test:server130/135、test:model39/39、test:store12/12(main 分支基线上即存在相同的 5+1 个存量失败,与本 PR 无关);tsc --noEmit无新增错误。