fix: Integer wrap in external-plugin review pair count causes out-of-bounds writes in ui_approve_tx

[cerberus-production]

Closes #1008

Summary

Automated security fix for Integer wrap in external-plugin review pair count causes out-of-bounds writes in ui_approve_tx (High).

CWE: CWE-787
OWASP: A04:2021-Insecure Design
Fix Confidence: high

What Changed

The vulnerability is an integer overflow when computing pluginUiMaxItems from plugin-reported screen counts. Two uint8_t values (pluginFinalize.numScreens and pluginProvideInfo.additionalScreens) are added and stored in uint8_t pluginUiMaxItems without overflow checking. This can cause: (1) the sum itself to wrap past 255, and (2) downstream getNbPairs() in ui_approve_tx.c to wrap when adding up to 3 more items (from address, network, max fees) to the already-large value, leading to a small allocation but large iteration count in setTagValuePairs(), causing OOB writes to g_pairs.

The fix adds a validation check in logic_sign_tx.c at the point where pluginUiMaxItems is assigned. It computes the sum using uint16_t to prevent truncation, then rejects the transaction if the total exceeds 252 (leaving room for the 3 additional UI pairs that getNbPairs may add, ensuring the downstream uint8_t accumulator in getNbPairs cannot exceed 255). This single check at the source prevents both the source overflow and the downstream accumulation overflow.

Caveats

• The cap of 252 is derived from the maximum of 3 additional pairs (from, network, max fees) that getNbPairs adds in the plugin path. If future code adds more fixed pairs to the plugin path without updating this limit, the downstream overflow could recur. A defense-in-depth approach would also widen the uint8_t counters in getNbPairs/setTagValuePairs, but that would require changing the ui_pairs_init signature and all its callers.
• Legitimate plugins requesting more than 252 screens will now be rejected. In practice, no known plugin uses anywhere near this many screens (internal plugins use at most ~5), so this is not a practical limitation.

Verification Checklist

• Review the code change
• Run tests to verify no regression
• Verify the vulnerability is addressed — already verified by Cerberus Sentinel

---

Created by Cerberus Merlin

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.10%. Comparing base (9373cce) to head (77f9112).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #1009   +/-   ##
========================================
  Coverage    56.10%   56.10%           
========================================
  Files           26       26           
  Lines         2417     2417           
  Branches       322      317    -5     
========================================
  Hits          1356     1356           
+ Misses        1057     1017   -40     
- Partials         4       44   +40     

Flag	Coverage Δ
unittests	56.10% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

elf sizes
source = source branch security/merlin-63c21dc
target = target branch develop

Device	.text source	.text target	.text delta	.bss source	.bss target	.bss delta	max stack size source	max stack size target	max stack size delta
stax	179966	179966	0	21794	21794	0	15064	15064	0
nanos2	114275	114275	0	20492	20492	0	20464	20464	0
nanox	114003	114003	0	20520	20520	0	8192	8192	0
flex	180045	180045	0	21794	21794	0	15064	15064	0
apex_p	159897	159897	0	21794	21794	0	19160	19160	0

Stack consumption summary (clone_app_stack_consumption)

⚠️ This summary is for informative purpose only. It may not give the application actual worst case, for example if the test coverage is low.

Device	Worst case (bytes)	Remaining stack (bytes)	Test
apex_p	1753	38603	test_clone.py::test_clone_thundercore[apex_p]
flex	1801	34459	test_clone.py::test_clone_thundercore[flex]
nanosp	1705	38651	test_clone.py::test_clone_thundercore[nanosp]
nanox	1697	6495	test_clone.py::test_clone_thundercore[nanox]
stax	1801	34459	test_clone.py::test_clone_thundercore[stax]

Full details

Stack consumption summary

⚠️ This summary is for informative purpose only. It may not give the application actual worst case, for example if the test coverage is low.

Device	Worst case (bytes)	Remaining stack (bytes)	Test
apex_p	3193	15967	test_eip712.py::test_eip712_batch[apex_p]
flex	3193	11871	test_eip712.py::test_eip712_batch[flex]
nanosp	3193	17271	test_eip712.py::test_eip712_batch[nanosp]
nanox	3193	4999	test_eip712.py::test_eip712_batch[nanox]
stax	3193	11871	test_eip712.py::test_eip712_batch[stax]

Full details