Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Don't access out of bounds array index: array[sizeof(array)] #14398

Merged
merged 1 commit into from Oct 8, 2018

Conversation

Projects
None yet
3 participants
@Empact
Copy link
Member

commented Oct 5, 2018

Split from #13525

@Empact Empact force-pushed the Empact:array-access branch Oct 5, 2018

@Empact Empact force-pushed the Empact:array-access branch to b09c814 Oct 5, 2018

@MarcoFalke MarcoFalke added the Tests label Oct 5, 2018

@MarcoFalke MarcoFalke changed the title Don't access out of bounds array index: array[sizeof(array)] tests: Don't access out of bounds array index: array[sizeof(array)] Oct 5, 2018

@practicalswift

This comment has been minimized.

Copy link
Member

commented Oct 5, 2018

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

This comment has been minimized.

Copy link
Member

commented Oct 6, 2018

Can you explain how this is oob?

@Empact

This comment has been minimized.

Copy link
Member Author

commented Oct 6, 2018

@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 (comment)

@MarcoFalke MarcoFalke merged commit b09c814 into bitcoin:master Oct 8, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Oct 8, 2018

Merge #14398: tests: Don't access out of bounds array index: array[si…
…zeof(array)]

b09c814 Don't access out of bounds array entry array[sizeof(array)] (Ben Woosley)

Pull request description:

  Split from #13525

Tree-SHA512: bf44609fc5d497cd1894b85dce33a55977b3e0f3d03b986333a85967c1f3aa89089461f830939072bbb4d2477ccce26b9caeb627215bfb86a331f58d3466a4bd

@Empact Empact deleted the Empact:array-access branch Dec 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.