Skip to content

fix(serve): 回环判定改为解析地址,请求行不再直进日志 - #26

Merged
modusensus merged 2 commits into
mainfrom
harden/serve-log-and-loopback
Sep 20, 2026
Merged

modusensus merged 2 commits into
mainfrom
harden/serve-log-and-loopback

Conversation

@modusensus

Copy link
Copy Markdown
Collaborator

两处都是上一轮安全审计里"属于加固而非漏洞"的两条,现在补上。方向一致:都不让一个不受信任的字符串来决定安全判定。

1) 回环判定改为解析地址,不再靠拼写

serve.host 能不能不带令牌绑定,只由 is_loopback_host 决定,而它原来是:

return name in ("localhost", "::1") or name.startswith("127.")

startswith("127.") 看着等价于 127.0.0.0/8,其实也匹配名字127.corp.example 是合法主机名,解析到哪里由它的所有者决定。放行它 = 无令牌把看板(服务器地址、登录用户、转发端口)绑到公网——而这正是 ensure_bindable 存在的唯一理由。

现在用 ipaddress 解析、取 is_loopback,名字只认 localhost

代价:两种"操作系统接受、解析器不认"的真回环写法现在算对外、要令牌——127.1(简写)与 localhost.(绝对形式)。判错的方向必须是"多要一个令牌"而不是"少要一个",所以这是故意的窄,README 与报错信息里都写明了。

2) 请求行不再直进日志

log_message(以及经由它进来的 log_error)把请求行原样写进日志记录。那一行由 BaseHTTPRequestHandler 按 latin-1 解码,所以客户端能塞进 ESC / NUL / DEL / C1 / 双向覆盖字符:足以让日志声称没发生过的事,或让 tail 它的终端渲染它从没收到过的输出。

现在非打印字符一律变成可见的 ?——留痕而不是抹掉,否则一次注入尝试会从日志里消失;行长限制在 500 字符,免得 64 KiB 的请求行变成 64 KiB 的日志行。

验证

  • pytest 412 passed(新增 21 个用例),覆盖率 86.72%;ruffmypy(本机与 --platform linux)与 _smoke_test.py 全过。
  • 新增的日志测试被证明能抓到那个漏洞:把 log_message 临时换回旧实现后它会失败,失败信息里原样出现了 ESC、NUL、DEL 与 C1 字符。
  • 回环边界用参数化用例锁住:127.corp.example / 127.0.0.1.example.com / 127.attacker.tld 判为对外,127.0.0.1 / [127.0.0.1] / [::1] / ::ffff:127.0.0.1 判为回环。

两处都来自安全审计,都在 `ponte serve` 这个面上,方向都是"别再失败开放":

- is_loopback_host 用 startswith("127.") 判定,于是也接受**主机名**:
  127.corp.example 是合法域名,解析到哪里由它的所有者决定。这个函数是
  ensure_bindable 的唯一判据,也就是看板(服务器地址、登录用户、转发端口)与
  网络之间的全部防线——放行这样一个名字等于无令牌把内网拓扑绑到公网。改为用
  ipaddress 解析地址、取 is_loopback,名字只认 localhost;操作系统接受而解析器
  不认的写法(127.1、localhost.)算作"对外",即宁可多要一个令牌。

- log_message(以及经由它进来的 log_error)把原始请求行直接写进日志记录。那一行
  由 BaseHTTPRequestHandler 按 latin-1 解码,所以客户端能塞进 ESC、NUL、DEL、
  C1 与双向覆盖字符:足以让日志声称没发生过的事,或让 tail 它的终端渲染它从没
  收到过的输出。现在非打印字符一律变成可见的 `?`(留痕而不是抹掉),行长也限制
  在 500 字符,免得 64 KiB 的请求行变成 64 KiB 的日志行。

测试:新增用例覆盖伪造字符与长度上限(把 log_message 换回旧实现时它会失败,已实测),
以及 127. 前缀主机名、127.1、localhost. 等边界写法。

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
Copilot AI lite review requested due to automatic review settings September 20, 2026 03:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: c44cae50-243c-4db8-a94d-d737a7ea459d


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

CI 的 3.11 腿抓到:is_loopback_host("::ffff:127.0.0.1") 在 3.11 上返回 False、
在 3.12 上返回 True——同一个写法,一个要令牌一个不要,而它决定的是看板能不能
在无令牌时绑到公网。原因是 IPv6Address.is_loopback 对 v4-mapped 形式的支持是
随版本加的,所以把令牌门禁建在它上面等于建在解释器版本上。

现在 v4-mapped 地址按它所代表的 IPv4 地址判定(::ffff:127.0.0.1 就是 127.0.0.1),
两个版本给出同一个答案。已在本地用模拟的 3.11 语义验证:新路径 True,旧路径 False。

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
Copilot AI review requested due to automatic review settings September 20, 2026 03:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@modusensus
modusensus merged commit dcbb3a0 into main Sep 20, 2026
11 checks passed
@modusensus
modusensus deleted the harden/serve-log-and-loopback branch September 20, 2026 03:25
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.

2 participants