Skip to content

fix(security): enforce base_path boundary in getActualPath and add S3 gateway authorization - #82

Open
Mcchen1008 wants to merge 1 commit into
OpenListTeam:mainfrom
Mcchen1008:fix/p0-basepath-escape-and-s3-authz
Open

Mcchen1008 wants to merge 1 commit into
OpenListTeam:mainfrom
Mcchen1008:fix/p0-basepath-escape-and-s3-authz

Conversation

@Mcchen1008

Copy link
Copy Markdown

概述

本 PR 修复代码审查中发现的 2 个 P0 级授权绕过漏洞,并附带 16 个回归测试。


P0-1:base_path 监禁可被 .. 段完全穿越(跨存储越权读/写/删)

根因:getActualPath()(src/backend/pkg/permission.ts)此前仅做字符串拼接,而下游 resolvePath() 的 .. 折叠只钳制到虚拟根 /——拼接上去的 base_path 前缀会被 .. 直接"弹掉":

base_path=/jail 的用户请求 "/../secret"
→ getActualPath: "/jail/../secret"(纯拼接)
→ resolvePath 折叠: "/secret"          ← 逃出监禁区,命中其他存储挂载

实测影响(低权限、无特权位、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/* 网关完全不校验权限(任意用户读写删全盘,含禁用用户)

根因(两个缺陷叠加):

  1. authUserFromReq()(src/backend/server/auth.ts)不拒绝 disabled 用户(与 getUserFromContext 不一致)——被禁用用户的存量 JWT 仍可通过 /s3、/dav(Bearer) 等入口;
  2. server/s3.ts 只检查"存在有效 JWT":不查权限位、不经 getActualPath() 收敛路径。

permission=0、base_path=/jail 的普通用户即可 GET/PUT/DELETE /s3/<任意挂载>/<key> 读写删整个虚拟文件系统。

修复:

  • authUserFromReq() 补 user.disabled 拒绝;
  • S3 全部操作路径经 getActualPath() 收敛到 base_path;
  • PutObject 要求 WRITE_CONTENT、DeleteObject 要求 DELETE 权限位(与 /api/fs/put、/api/fs/remove 对齐);
  • 防御性拒绝 guest。

附带修复

  • 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:server 130/135、test:model 39/39、test:store 12/12(main 分支基线上即存在相同的 5+1 个存量失败,与本 PR 无关);tsc --noEmit 无新增错误。

… 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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant