fix: validate embedding ids unconditionally in CPU kernel to prevent OOB read (CWE-125)#79507
fix: validate embedding ids unconditionally in CPU kernel to prevent OOB read (CWE-125)#79507Pandya-mayur wants to merge 2 commits into
Conversation
The id-range check in EmbeddingCPUFunctor::apply was nested inside `if (padding_idx_ == kNoPadding && ids[i] != padding_idx_)`. Because kNoPadding is -1, creating an embedding layer with any padding_idx (e.g. the common padding_idx=0) skipped the bounds check for every id, and the copy loop then indexed the weight table with an unvalidated, attacker-controlled id, causing an out-of-bounds read (heap disclosure or crash). Make the range check (0 <= id < row_number) unconditional; only the padding-row zeroing remains conditional on padding_idx. Ref: PaddlePaddle#79491
|
你的PR提交成功,感谢你对开源项目的贡献! |
risemeup1111
left a comment
There was a problem hiding this comment.
已发现需要修复的问题,细节已写在 inline review comments 里。
| row_number, | ||
| ids[i])); | ||
| } | ||
| PADDLE_ENFORCE_LT( |
There was a problem hiding this comment.
这次把检查条件改对了,但没有补回归测试。现有 test_ai_nn_functional_input.py / test_ai_embedding_onehot.py 只覆盖合法 padding_idx、负 padding_idx 和 padding_idx 越界,没有覆盖“padding_idx 已设置时输入 id 越界”的 CPU 路径。没有这个 case,后续很容易把这段保护逻辑改回去,CI 也拦不住这类 OOB 回归。
建议补一个 CPU 动态图用例,至少验证 padding_idx=0 时越界 id 会抛 ValueError,例如:
paddle.disable_static(paddle.CPUPlace())
x = paddle.to_tensor([0, 6], dtype='int64')
w = paddle.randn([5, 3])
with self.assertRaises(ValueError):
paddle.nn.functional.embedding(x, w, padding_idx=0)处理要求:请针对该评论修复并提交新的 commit。
Verifies that with padding_idx set, an out-of-range (or negative) input id raises ValueError on the CPU kernel instead of reading out of bounds. Forces the CPU place so the test is independent of the build target.
risemeup1111
left a comment
There was a problem hiding this comment.
已复查新增 commit,CPU 越界 id 的回归测试已经补上,之前的 P1 现在看起来已覆盖。当前未发现需要阻塞合入的问题,细节见之前的 inline review comments。
|
Thanks for the review — addressed the P1.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #79507 +/- ##
===========================================
Coverage ? 100.00%
===========================================
Files ? 1
Lines ? 2
Branches ? 0
===========================================
Hits ? 2
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
PR Category
Operator Mechanism
PR Types
Security
Description
Fixes #79491 — out-of-bounds read in the CPU embedding kernel when
padding_idxis set.Problem
In
paddle/phi/kernels/cpu/embedding_kernel.cc,EmbeddingCPUFunctor::applyvalidates each id only inside:kNoPaddingis-1, so as soon as an embedding layer is created with anypadding_idx(e.g. the very commonpadding_idx=0used in NLP models), the entire range check is skipped for every id. The copy loop then indexes the weight table with an unvalidated, attacker-controlledids[i]:40on a 16-row table) returns raw heap bytes past the weight matrix — information disclosure.1e9) crashes the process with SIGSEGV inEmbeddingCPUFunctor::apply.A reproducible PoC and an AddressSanitizer trace (
heap-buffer-overflow READ) are in issue #79491.Solution
0 <= id < row_numberunconditional for every id.padding_idx.Valid inputs are unaffected (padding indices are always in range); only genuinely out-of-range ids are now rejected with the existing
InvalidArgumenterror instead of reading out of bounds.This vulnerability has also been responsibly disclosed to the Baidu security team (international_bsrc@baidu.com).
cc @luotao1
是否引起精度变化
否