Skip to content

Deserialize limit#870

Merged
shargon merged 3 commits intoneo-project:masterfrom
shargon:limit-deserialize
Jun 26, 2019
Merged

Deserialize limit#870
shargon merged 3 commits intoneo-project:masterfrom
shargon:limit-deserialize

Conversation

@shargon
Copy link
Copy Markdown
Member

@shargon shargon commented Jun 26, 2019

Before the maximum was 16777216
But in ExecutionEngine is 1048576

@shargon shargon requested a review from erikzhang June 26, 2019 12:12
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jun 26, 2019 •
edited
Loading

Codecov Report

Merging #870 into master will not change coverage.
The diff coverage is 80%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #870   +/-   ##
=======================================
  Coverage   43.53%   43.53%           
=======================================
  Files         177      177           
  Lines       12566    12566           
=======================================
  Hits         5471     5471           
  Misses       7095     7095
Impacted Files Coverage Δ
neo/SmartContract/InteropService.cs 20.98% <0%> (ø) ⬆️
neo/SmartContract/Helper.cs 66.82% <100%> (ø) ⬆️
...eo/SmartContract/Native/Tokens/Nep5AccountState.cs 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 897df9c...a99c968. Read the comment docs.

@erikzhang
Copy link
Copy Markdown
Member

erikzhang commented Jun 26, 2019

They have already been limited.

neo/neo/SmartContract/InteropService.cs

Lines 211 to 212 in 897df9c

if (serialized.Length > engine.MaxItemSize)
return false;

neo/neo/SmartContract/InteropService.cs

Line 233 in 897df9c

item = engine.CurrentContext.EvaluationStack.Pop().GetByteArray().DeserializeStackItem(engine.MaxArraySize);

@shargon
Copy link
Copy Markdown
Member Author

shargon commented Jun 26, 2019

This solve the problem in deserialization, because the binary data could be "hand made"

shargon
shargon commented Jun 26, 2019
View reviewed changes
@shargon shargon merged commit 0c0e895 into neo-project:master Jun 26, 2019
@shargon shargon deleted the limit-deserialize branch June 26, 2019 14:33
vncoelho
vncoelho reviewed Jun 26, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Good protections!

@shargon shargon mentioned this pull request Dec 16, 2019
Merged
Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020
@Celia18305
fix links (neo-project#870)
13fab70
Tommo-L pushed a commit to Tommo-L/neo that referenced this pull request Jun 22, 2020
@shargon @Tommo-L
Deserialize limit (neo-project#870)
5a54eae 
* Deserialize limit

* Delete optional parameters

* Use `ExecutionEngine.MaxSizeForBigInteger`
@roman-khimov roman-khimov mentioned this pull request Oct 7, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vncoelho vncoelho vncoelho left review comments

@erikzhang erikzhang erikzhang approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@shargon @codecov-io @erikzhang @vncoelho