Skip to content

Conversation

@pawanjay176
Copy link
Contributor

Buffer allocated in visit_seq does not have an upper limit. Limit buffer capacity to 4096.

Copy link
Contributor

@romanb romanb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify, I'm not so familiar with the serialisation code: Since this is just for Vec::with_capacity, this limits only the initial capacity of the buffer, so that you cannot make the deserialisation code allocate an arbitrarily large Vec without actually "sending" that much data (e.g. if the Multiaddr is received over the network), since the Vec will still grow beyond its capacity as necessary. Correct?

@pawanjay176
Copy link
Contributor Author

@romanb yes that is correct. Without a limit, you can send malformed input such that seq.size_hint() returns a very large value and crashes the program even if the following data is pretty small.

@mxinden mxinden merged commit f7ab4f7 into libp2p:master Nov 14, 2020
@mxinden
Copy link
Member

mxinden commented Nov 14, 2020

Thanks @pawanjay176!

@mxinden
Copy link
Member

mxinden commented Nov 14, 2020

I pushed 2e6b0ec to update patch version and changelog. Should we do a release already, or wait till the next larger release?

@romanb
Copy link
Contributor

romanb commented Nov 14, 2020

No reason to wait, a multiaddr patch release seems appropriate.

@mxinden
Copy link
Member

mxinden commented Nov 14, 2020

For the record parity-multiaddr v0.9.5 is now released and published.

@pawanjay176 pawanjay176 deleted the multiaddr-fix branch November 15, 2020 17:20
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.

3 participants