Split from #13525
tests: Don't access out of bounds array index: array[sizeof(array)] #14398
pull Empact wants to merge 1 commits into bitcoin:master from Empact:array-access changing 2 files +6 −6-
Empact commented at 7:46 AM on October 5, 2018: member
- Empact force-pushed on Oct 5, 2018
-
Don't access out of bounds array entry array[sizeof(array)] b09c81454e
- Empact force-pushed on Oct 5, 2018
- MarcoFalke added the label Tests on Oct 5, 2018
- MarcoFalke renamed this:
Don't access out of bounds array index: array[sizeof(array)]
tests: Don't access out of bounds array index: array[sizeof(array)]
on Oct 5, 2018 -
practicalswift commented at 7:56 AM on October 5, 2018: contributor
Concept ACK
The fix seems to be exhaustive:
$ git grep -E '&[a-zA-Z0-9_:]*\[sizeof\([a-zA-Z0-9_:]*\)\]' src/bench/checkblock.cpp: (const char*)&block_bench::block413567[sizeof(block_bench::block413567)], src/bench/checkblock.cpp: (const char*)&block_bench::block413567[sizeof(block_bench::block413567)], src/test/script_tests.cpp: BOOST_CHECK(EvalScript(directStack, CScript(&direct[0], &direct[sizeof(direct)]), SCRIPT_VERIFY_P2SH, BaseSignatureChecker(), SigVersion::BASE, &err)); src/test/script_tests.cpp: BOOST_CHECK(EvalScript(pushdata1Stack, CScript(&pushdata1[0], &pushdata1[sizeof(pushdata1)]), SCRIPT_VERIFY_P2SH, BaseSignatureChecker(), SigVersion::BASE, &err)); src/test/script_tests.cpp: BOOST_CHECK(EvalScript(pushdata2Stack, CScript(&pushdata2[0], &pushdata2[sizeof(pushdata2)]), SCRIPT_VERIFY_P2SH, BaseSignatureChecker(), SigVersion::BASE, &err)); src/test/script_tests.cpp: BOOST_CHECK(EvalScript(pushdata4Stack, CScript(&pushdata4[0], &pushdata4[sizeof(pushdata4)]), SCRIPT_VERIFY_P2SH, BaseSignatureChecker(), SigVersion::BASE, &err)); -
MarcoFalke commented at 3:36 AM on October 6, 2018: member
Can you explain how this is oob?
-
Empact commented at 4:52 AM on October 6, 2018: member
@MarcoFalke arrays of defined size N are bounded from 0 to N-1, while these index at N. Not directly harmful as only the addresses are taken, but invalid in the sense that they exceed the deined bounds. Suggested by @practicalswift here: #13525 (review)
- MarcoFalke merged this on Oct 8, 2018
- MarcoFalke closed this on Oct 8, 2018
- MarcoFalke referenced this in commit 2a747337ae on Oct 8, 2018
- Empact deleted the branch on Dec 10, 2018
- Munkybooty referenced this in commit bb24cd7144 on Jul 21, 2021
- Munkybooty referenced this in commit 8efb90f882 on Jul 22, 2021
- DrahtBot locked this on Sep 8, 2021
