Skip to content

fix(ticket): validate the miner address ownership on ticket open - #1336

Closed
vipwzw wants to merge 5 commits into
masterfrom
fix/ticket-open-miner-auth
Closed

vipwzw wants to merge 5 commits into
masterfrom
fix/ticket-open-miner-auth

Conversation

@vipwzw

@vipwzw vipwzw commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

问题

TicketOpen 只校验「签名者 == ReturnAddress」,或「签名者是 ReturnAddress 绑定的矿工」,对 MinerAddress 本身没有任何归属校验(也没有地址合法性校验):

if action.fromaddr != topen.ReturnAddress {   // MinerAddress 的校验只在这个分支里
    mineraddr := action.getBind(topen.ReturnAddress)
    if mineraddr != action.fromaddr { return nil, ty.ErrMinerNotPermit }
    if topen.MinerAddress != mineraddr { return nil, ty.ErrMinerAddr }
}

而 MinerAddress 正是票据的归属标签:

  • 本地索引 LODB-ticket-tl:<MinerAddress>:<status>:<ticketId>(ReceiptTicket.Addr = MinerAddress);
  • 钱包 getTicketsByStatus 按钱包账户地址(即 MinerAddress)取票;
  • 共识 searchTargetTicket 只要求 privmap[MinerAddress] 存在。

于是任何签名者只要把自己的地址填进 ReturnAddress(最常见的自挖写法),就能把票据挂到任意矿工名下:该矿工节点会把票据当成自己的票去挖,而出块奖励由 ExecDepositFrozen 增发到票据的 ReturnAddress(出票人)——受害矿工付出算力与抽签机会,收益归攻击者;攻击者成本仅为票面价(关闭票据后原路退回)。注入票还会污染受害矿工的本地索引,形成链上归因伪造。

同一函数还有 Count > len(PubHashes) 的越界访问:只检查了 len(PubHashes) == 0 就按 Count 下标取值,index out of range panic(被 DriverBase.Exec 的 recover 兜成 ErrActionNotSupport)。

修复

修复分两层,互相补充。

1. 执行器层:新增 dapp 分叉 ticket.ForkTicketMinerAuth(默认关闭)

该分叉默认不开启:InitFork 把 ForkTicketMinerAuth 注册为 types.MaxHeight,只有链配置 [fork.sub.ticket] ForkTicketMinerAuth 显式指定激活高度后才启用。原因:链配置里没有 [fork] 段时(例如部署仓的 bityuan.toml)initForkConfig 会被跳过、分叉按注册默认值生效;若默认注册为 0,升级二进制就会立刻改变这类链的历史区块重放与当下挖矿行为。

  1. Action.TicketOpen 强制 address.CheckAddress(MinerAddress, height);
  2. 要求 MinerAddress ∈ {fromaddr, getBind(ReturnAddress)},否则返回 ErrMinerAddr:只能挂在自己名下,或挂在 ReturnAddress 已通过 Tbind 授权的矿工名下。原有「非 ReturnAddress 签名者必须是该 ReturnAddress 绑定矿工」的逻辑保持不变,自挖(buyTicketOne)与冷地址绑定代挖(buyMinerAddrTicketOne)不受影响;
  3. PubHashes 改为按 Count 校验,返回 ErrOpenTicketPubHash 而不是 panic。这条不需要分叉保护:panic 交易会被 PreExecBlock 从区块中丢弃,从未写入任何区块。

分叉高度:主链 chain33.fork.toml(及镜像主链的 plugin/dapp/evm/cmd/ci2/chain33.proxyminer.toml)为 48100000,按仓库既有做法取 tip + 241920 ≈ 两周升级窗口;平行链 chain33.para.toml 及 readme、执行器 testdata、rpc 测试内嵌配置为 0。未显式配置该键的链保持关闭,升级二进制本身不改变行为。

2. 矿工侧(无需分叉):ReturnAddress 白名单过滤选票

执行器规则只能挡住「未绑定」的裸挂:Tbind 由票主(ReturnAddress) 单方签名,矿工本人不签名,因此攻击者可以自签 Tbind{ReturnAddress=自己, MinerAddress=受害矿工} 再挂票,通过执行器的绑定分支(已知限制,见下)。这类票的危害发生在选票/出块环节,所以在共识取票的唯一入口做本地过滤,不改链上状态、不需要分叉:

  • plugin/dapp/ticket/wallet/exec.go 的 On_WalletGetTickets → filterTrustedTickets(共识 flushTicket 通过 WalletGetTickets 取票的唯一入口);
  • 只保留 MinerAddress == ReturnAddress(自挖)或 ReturnAddress 命中 wallet.sub.ticket.minerwhitelist 的票,被过滤的票打 skipUntrustedTicket 告警;
  • minerwhitelist 就是矿工本地的批准名单:只有名单内的 ReturnAddress 才能使用本矿工的挖矿身份。默认 ["*"] 表示不过滤,保持原有挖矿行为;链上不新增批准动作;
  • 局限:只保护升级并配置了白名单的节点;不阻止攻击者把票发上链,也不清除链上票据/本地索引(TicketList 仍能查到),「链上归因伪造」仍在。它解决的是算力被占用、奖励被拿走这一核心危害。

测试

plugin/dapp/ticket/executor/ticket_miner_auth_test.go(8 个用例):

测试 覆盖
TestForkTicketMinerAuthDefaultDisabled 没有 [fork] 段的链配置下分叉默认关闭(types.MaxHeight),升级不改变旧行为
Test_TicketOpen_RejectsForeignMinerAddress 第三方把票挂到别的矿工 → ErrMinerAddr,且校验发生在冻结之前(票面资金未被动用)
Test_TicketOpen_AllowsOwnMinerAddress 自挖路径照常 ExecOk,票据以自己为 MinerAddress 落地
Test_TicketOpen_AllowsBoundMinerAddress Tbind 授权后,票主开票与绑定矿工代开票两条路径都放行
Test_TicketOpen_RejectsInvalidMinerAddress 非法 MinerAddress 被地址校验拒绝
Test_TicketOpen_RejectsPubHashesCountMismatch Count > len(PubHashes) 返回 ErrOpenTicketPubHash,不再 panic
Test_TicketOpen_LegacyPathBeforeFork 分叉高度之前保持旧行为(说明必须留足升级窗口)
Test_TicketOpen_SelfTbindStillInjects 已知限制:自签 Tbind 仍可挂票,由第 2 条的本地白名单兜底

plugin/dapp/ticket/wallet/ticket_test.go:TestFilterTrustedTickets(默认 ["*"] 不过滤;显式白名单时只保留自挖票与白名单 ReturnAddress 的票)。

回归:plugin/dapp/ticket/... 与 plugin/consensus/ticket/... 全绿。

Review 需注意

  • 分叉默认关闭:未显式配置 ForkTicketMinerAuth 的链升级二进制后行为不变。主链请在 48100000 生效前完成历史扫描——若历史上已存在「第三方把票挂到别的矿工名下」的交易,分叉生效后重放会失败。
  • 矿工侧过滤是本地缓解:minerwhitelist 默认为 ["*"],需要矿工显式配置才生效;它不替代链上规则,也不解决链上归因伪造。
  • 已知限制:Tbind 是票主单方签名,不是矿工侧批准;要链上强制「矿工批准」需新增矿工签名的 action + 新分叉 + 钱包支持,不在本 PR 范围。
  • 分叉前预埋的 3 段 ticketId 旧票仍可被挖(其 pubHash 校验恒被跳过),矿工侧白名单过滤可兜底;建议后续在分叉后拒绝非创世的 3 段 ticketId。
  • Exec_Genesis 缺少 height != 0 校验导致的分叉后重入(同一份分析文档 §3)是独立缺陷,由另一个提交修复,与本 PR 互不依赖。
  • 分析与回归细节见 docs/security/ticket-topen-mineraddress-injection-analysis.md §8。

king added 4 commits October 15, 2026 18:30
TicketOpen 只校验了签名者与 ReturnAddress 的一致性, 对 MinerAddress 本身没有任何
归属校验。而 MinerAddress 正是票据的归属标签: 本地索引 LODB-ticket-tl:<MinerAddress>:<status>、
钱包取票、共识选票都按它来。于是任何签名者只要把自己的地址填进 ReturnAddress, 就能把
票据挂到任意矿工名下 —— 该矿工节点会把它当成自己的票挖出来 (Exec_Miner 只要求签名者是
MinerAddress 或 ReturnAddress), 出块奖励却由 ExecDepositFrozen 增发到票据的 ReturnAddress
(出票人), 等于无偿占用受害矿工的算力与抽签机会; 票据还会污染该矿工的本地索引, 形成
链上归因伪造。攻击者成本仅为票面价 (关闭票据后原路退回)。

同一函数还有 Count > len(PubHashes) 的越界访问: 只检查了 len == 0 就按 Count 下标取值。

修复, 全部落在新增 dapp 分叉 ticket.ForkTicketMinerAuth (InitFork 默认注册 0):

- TicketOpen 强制 address.CheckAddress(MinerAddress, height);
- 并要求 MinerAddress ∈ {fromaddr, getBind(ReturnAddress)}: 只能挂在自己名下, 或挂在
  ReturnAddress 已通过 Tbind 明确授权的矿工名下, 否则返回 ErrMinerAddr;
- 原有 "非 ReturnAddress 签名者必须是该 ReturnAddress 绑定矿工" 的逻辑不变, 因此
  自挖 (buyTicketOne) 与冷地址绑定代挖 (buyMinerAddrTicketOne) 不受影响;
- PubHashes 改为按 Count 校验, 返回 ErrOpenTicketPubHash 而不是 panic; 该修复不需要
  分叉保护 —— panic 交易会被 PreExecBlock 从区块里丢弃, 从未写入任何区块, 不影响重放;
- 分叉高度: 主链 chain33.fork.toml / 镜像主链的 proxyminer 配置为 48100000
  (按仓库既有做法取 tip + 241920 ≈ 两周升级窗口), 平行链 chain33.para.toml 及
  文档/测试内嵌配置为 0。
ForkTicketMinerAuth 生效后的回归用例 (6 个):

- Test_TicketOpen_RejectsForeignMinerAddress: 第三方把票挂到别的矿工名下返回
  ErrMinerAddr, 且校验发生在冻结之前, 票面资金不被动用;
- Test_TicketOpen_AllowsOwnMinerAddress: 自挖路径 (MinerAddress = ReturnAddress =
  签名者) 照常 ExecOk, 票据以自己为 MinerAddress 落库;
- Test_TicketOpen_AllowsBoundMinerAddress: 票主开票给 Tbind 已授权的矿工、以及
  该矿工代开票, 两条路径都放行;
- Test_TicketOpen_RejectsInvalidMinerAddress: 非法 MinerAddress 被地址校验拒绝;
- Test_TicketOpen_RejectsPubHashesCountMismatch: Count > len(PubHashes) 返回
  ErrOpenTicketPubHash, 不再 index out of range panic;
- Test_TicketOpen_LegacyPathBeforeFork: 分叉高度之前保持旧行为, 说明
  ForkTicketMinerAuth 必须配置在留足升级窗口的未来高度。
新增 §8 记录 H-2 (TicketOpen 不校验 MinerAddress 归属) 的修复实现: 分叉名与门控范围、
TicketOpen 的归属校验规则、PubHashes 越界修复、主链/平行链分叉高度、6 个回归用例;
§1 结论与 §7 验证范围同步补充修复状态, 并注明 §3 的 Exec_Genesis 重入由独立提交修复。
解决与 master(v1.76.1/v1.76.3) 的冲突,合并点 06858b7。

冲突原因:master 已先合入同期安全修复,在相同位置新增了 fork 配置与
ticket PoC 用例,与本 PR 的 ForkTicketMinerAuth 改动冲突。

- 配置:master 的 ForkTicketCloseDup/ForkMinerExecerCheck 与本 PR 的
  ForkTicketMinerAuth 共存于所有跟踪的 [fork.sub.ticket] 配置
  (chain33.fork.toml、chain33.para.toml、chain33.para.toml.readme、
  ci2 proxyminer、执行器 testdata、rpc 内嵌配置);
  主链 ForkTicketMinerAuth 仍为 48100000,平行链/测试配置为 0。
- ticket_miner_inject_poc_test.go:master 已入库的 H-2 注入 PoC 断言的是修复前
  行为,合并后会被 ErrMinerAddr 提前挡下。演示“注入成功”的用例改为显式使用
  ForkTicketMinerAuth 未生效的配置(pocCfgBeforeMinerAuth)并断言该前提,
  继续固定修复前行为;Count > len(PubHashes) 的修复不设分叉,该用例改为断言
  返回 ErrOpenTicketPubHash 且不再 panic。
- docs:同步 PoC 用例改名(Test_TicketGenesisReRunRejected_PostFork、
  Test_TicketOpen_CountExceedsPubHashesRejected)与合并后的验证范围(§7/§8)。

ticketdb.go 由 git 自动合并,MinerAddress 归属校验与 PubHashes 越界修复均保留。

验证:go test ./plugin/dapp/ticket/... ./plugin/consensus/ticket/... 全绿;
gofmt/git diff --check 干净。
@vipwzw

vipwzw commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

已合并 master(v1.76.3, 06858b7)解决冲突,合并提交 facad9ef8。冲突集中在两处:

1. [fork.sub.ticket] 配置
master 已先合入同期修复,新增 ForkTicketCloseDup / ForkMinerExecerCheck,与本 PR 的 ForkTicketMinerAuth 落在同一位置。合并结果保留全部三条:主链 ForkTicketMinerAuth=48100000,平行链与测试配置为 0;所有含 [fork.sub.ticket] 的跟踪配置(chain33.fork.toml、chain33.para.toml、chain33.para.toml.readme、ci2 proxyminer、执行器 testdata、rpc 内嵌配置)均已补齐。

2. ticket_miner_inject_poc_test.go(语义冲突)
master 已入库的 H-2 PoC 断言的是修复前行为,与本 PR 的修复冲突,处理如下:

  • 演示“注入成功”的 3 个用例(Test_TicketOpen_InjectsTicketsToArbitraryMiner、Test_InjectedTicket_NotMinableByVictim_PostFork、Test_InjectedTicket_MinedByVictim_PreFork_RewardGoesToAttacker)改为显式使用 ForkTicketMinerAuth 尚未生效的配置(pocCfgBeforeMinerAuth)并断言该前提,继续固定修复前的旧行为;修复后的拒绝/放行路径仍由 ticket_miner_auth_test.go 覆盖。
  • Count > len(PubHashes) 的修复不设分叉保护,该用例改为断言返回 ErrOpenTicketPubHash 且不再 panic,并改名为 Test_TicketOpen_CountExceedsPubHashesRejected。
  • 分析文档 §7/§8 同步用例改名与合并后的验证范围。

本地验证:go test ./plugin/dapp/ticket/... ./plugin/consensus/ticket/... 全绿;gofmt、git diff --check 干净。

…kets

按 review 意见收口 H-2(TicketOpen 不校验 MinerAddress 归属):

1. ForkTicketMinerAuth 在 InitFork 中改为注册 types.MaxHeight(默认关闭)。
   链配置里没有 [fork] 段时(例如部署仓的 bityuan.toml)initForkConfig 会被跳过,
   分叉按注册默认值生效;默认注册为 0 会让升级二进制立即改变这类链的历史重放与
   当下挖矿行为。必须由 [fork.sub.ticket] ForkTicketMinerAuth 显式指定激活高度。
   新增 TestForkTicketMinerAuthDefaultDisabled。

2. 矿工侧(无需分叉):On_WalletGetTickets 是共识 flushTicket 取票的唯一入口,
   在这里按 wallet.sub.ticket.minerwhitelist 过滤 ReturnAddress:只保留自挖票
   (MinerAddress == ReturnAddress) 与白名单内的票,被过滤的票打告警。
   minerwhitelist 即矿工本地的批准名单;默认 ["*"] 不过滤,保持原有挖矿行为。
   新增 TestFilterTrustedTickets,chain33.toml 补充该配置项语义注释。

3. 明确 Tbind 不能作为矿工侧批准:TicketBind 只要求 fromaddr == ReturnAddress,
   即票主单方签名,矿工本人不签名;攻击者可自签 Tbind 把 ReturnAddress 绑到任意
   矿工,从而通过执行器的绑定分支。新增 Test_TicketOpen_SelfTbindStillInjects
   固定该已知限制,由第 2 条的本地白名单兜底。

4. 文档 §7/§8 同步:默认关闭、两层修复的分工、白名单作为批准名单、已知限制与
   测试清单。

验证:go test ./plugin/dapp/ticket/... ./plugin/consensus/ticket/... 全绿;
gofmt -s 与 git diff --check 干净。
@vipwzw vipwzw closed this Oct 19, 2026
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