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

enforce canonical encoding and MAX_SIZE in VarIntSerializer #216

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dgpv
Copy link
Contributor

@dgpv dgpv commented Oct 22, 2019

core.serialize.VarIntSerializer now checks for value bounds of deserialized
compact size integer. If it enconters non-canonical encoding, or the size
bigger than MAX_SIZE (0x02000000) it throws DeserializationValueBoundsError

I just noticed that VarIntSerializer.stream_deserialize() is not in line with current Core's ReadCompactSize(). It does not check for MAX_SIZE (the check that was there from the start, I think), and it also does not check for the encodings to be canonical (if the encoding is of 3-byte length, it should not contain value less than 0xFD, etc) - the constraint added in 2013.

This patch adds the checks and the new exception, DeserializationValueBoundsError.

Tests now do not try to validate non-canonical encodings, but check that non-canoncial encodings throw the appropriate exception.

`core.serialize.VarIntSerializer` now checks for value bounds of deserialized
compact size integer. If it enconters non-canonical encoding, or the size
bigger than `MAX_SIZE` (0x02000000) it throws DeserializationValueBoundsError
@dgpv
Copy link
Contributor Author

dgpv commented Oct 22, 2019

the use of int() may be a problem on python2, but as far as I remember, python-bitcoinlib is announced to be python3-only from the next release.

@petertodd
Copy link
Owner

Yeah, writing py3-only code is fine now.

Is there anything else you'd do if this pull-req was py3-only?

@dgpv
Copy link
Contributor Author

dgpv commented Oct 31, 2019

Is there anything else you'd do if this pull-req was py3-only?

No, this code was ported from python-bitcointx, which is py3-only from the start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants