Securize ConsensusContext Deserialization#618
Conversation
jsolman
left a comment
There was a problem hiding this comment.
I originally had some safeguards like this; however Erik made the point that it wasn’t necessary for the consensus context, since the consensus context itself is never sent over the network it is only serialized and deserialized locally from the same node. So there should not be any vulnerability by not checking these maximums on deserialization.
|
This kind of checks is necessary, there is no reason for don't establish a safe maximum, although the process is trustable |
|
While, I don’t think this fixes any actual security vulnerability in this case, I agree with being defensive on the deserialization and having the safeguards, so I approved. It is a general good practice to always validate input. |
igormcoelho
left a comment
There was a problem hiding this comment.
I agree that this should become the standard practice. Although not necessary in this situation, it also helps semantically understanding the maximum limit on each field.
* 2.9.0 * updates for 2.9.0 * Update v2.9.0.md (neo-project#610) Adjustment for instruction of getting nep-5 applicationlog. * Update invokescript.md (neo-project#613) Add tx * Update invokefunction.md (neo-project#612) Add tx for response. * Create getwalletheight (neo-project#611) Add getwalletheight api. * updates for 2.9.0 * minor updates * Update setup.md (neo-project#617) Add introduction of Plugins. * Update v2.9.0.md (neo-project#615) Add introduction for setting config.json * final updates * Update v2.9.0.md (neo-project#618) Add notes for install plugins. * plugin related
No description provided.