Fix invalid memory access in CScript::operator+= (guidovranken, ajtowns)#11284
Merged
laanwj merged 1 commit intobitcoin:masterfrom Oct 2, 2017
Merged
Fix invalid memory access in CScript::operator+= (guidovranken, ajtowns)#11284laanwj merged 1 commit intobitcoin:masterfrom
laanwj merged 1 commit intobitcoin:masterfrom
Conversation
Contributor
|
Concept ACK. Nice! |
Contributor
|
ACK - good spot. |
dcousens
approved these changes
Sep 11, 2017
sipa
reviewed
Sep 11, 2017
Member
sipa
left a comment
sipa
left a comment
There was a problem hiding this comment.
utACK 84a8b8ab28cf379f1909f0bfc63c9eb25fba4dbd
I was going to suggest just outlawing adding a CScript to itself, but this fix is so small there's no point doing anything else.
src/script/script.h
Outdated
Contributor
Author
There was a problem hiding this comment.
Ooops. Fixed. Ran the clang-format snippet over it; also changed the declaration of "hex" in the test code.
ajtowns
force-pushed
the
cscript_insert
branch
from
84a8b8a to
d601f16
Compare
Contributor
Author
|
Testing with the updated script_tests.cpp -- without reserve(): with reserve(): |
Contributor
|
I believe common practice would be to credit the reporter (@guidovranken) in the commit message. |
maflcko
changed the title
Member
|
utACK d601f16 |
laanwj
added a commit
that referenced
this pull request
Oct 2, 2017
…vranken, ajtowns) d601f16 Fix invalid memory access in CScript::operator+= (Anthony Towns) Pull request description: This is a fix for #11114 -- invoking "s += s" gets turned into "s.insert(s.end(), s.begin(), s.end())" which can result in an invalid memory access is s.capacity() < 2*s.size() (because s gets resized and possibly moved, so s.begin() and s.end() become invalid references when reading the values to be appended). The fix is straightforward: reserve enough space in advance, so that insert() doesn't need to resize and thus its arguments remain valid. A simple test case is added as well; though you probably need to run it via valgrind to actually catch the problem when it's not fixed... Tree-SHA512: 4720d0c17463fdc43b344c45fe603423d20b30d48da1b9d85eeedc505d7f34db1ed5495ef1556459ae962a94717e3c6e8fc441763771901efea210d01322b7ef
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 31, 2020
… (guidovranken, ajtowns) d601f16 Fix invalid memory access in CScript::operator+= (Anthony Towns) Pull request description: This is a fix for bitcoin#11114 -- invoking "s += s" gets turned into "s.insert(s.end(), s.begin(), s.end())" which can result in an invalid memory access is s.capacity() < 2*s.size() (because s gets resized and possibly moved, so s.begin() and s.end() become invalid references when reading the values to be appended). The fix is straightforward: reserve enough space in advance, so that insert() doesn't need to resize and thus its arguments remain valid. A simple test case is added as well; though you probably need to run it via valgrind to actually catch the problem when it's not fixed... Tree-SHA512: 4720d0c17463fdc43b344c45fe603423d20b30d48da1b9d85eeedc505d7f34db1ed5495ef1556459ae962a94717e3c6e8fc441763771901efea210d01322b7ef
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a fix for #11114 -- invoking "s += s" gets turned into "s.insert(s.end(), s.begin(), s.end())" which can result in an invalid memory access is s.capacity() < 2*s.size() (because s gets resized and possibly moved, so s.begin() and s.end() become invalid references when reading the values to be appended).
The fix is straightforward: reserve enough space in advance, so that insert() doesn't need to resize and thus its arguments remain valid.
A simple test case is added as well; though you probably need to run it via valgrind to actually catch the problem when it's not fixed...