Skip to content

Fix unsafe memory access#376

Closed
shargon wants to merge 1 commit intoNeo.Actorfrom
fix-memory-access
Closed

Fix unsafe memory access#376
shargon wants to merge 1 commit intoNeo.Actorfrom
fix-memory-access

Conversation

@shargon
Copy link
Copy Markdown
Member

@shargon shargon commented Sep 13, 2018 •
edited
Loading

Without checking the length, is possible to read arbitrary memory

  • Should be merged to master

@shargon
Fix unsafe memory access
7ef460f 
Is possible to read arbitrary memory
@shargon shargon requested a review from erikzhang September 13, 2018 10:43
@erikzhang
Copy link
Copy Markdown
Member

erikzhang commented Sep 13, 2018 •
edited
Loading

These unsafe methods are designed for performance, so some checks are omitted. They require developers to use them carefully and are not public.

@shargon
Copy link
Copy Markdown
Member Author

shargon commented Sep 13, 2018

I think that the length should be checked, is easy to forget the previous check, for example in this:

neo/neo/Wallets/SQLite/UserWallet.cs

Lines 34 to 43 in aece82b

get
{
byte[] buffer = LoadStoredData("Version");
if (buffer == null) return new Version(0, 0);
int major = buffer.ToInt32(0);
int minor = buffer.ToInt32(4);
int build = buffer.ToInt32(8);
int revision = buffer.ToInt32(12);
return new Version(major, minor, build, revision);
}

@erikzhang
Copy link
Copy Markdown
Member

erikzhang commented Sep 13, 2018 •
edited
Loading

We need these high performance methods in some places.

For example:

neo/neo/Cryptography/Murmur3.cs

Line 35 in b8ad89e

uint k = array.ToUInt32(i);

We can find out where the methods are misused and then use BitConverter.ToXXX() instead.

@shargon shargon mentioned this pull request Sep 14, 2018
Merged
@shargon
Copy link
Copy Markdown
Member Author

shargon commented Sep 14, 2018

I will search and fix where the length is not checked, and close this issue

@shargon shargon mentioned this pull request Sep 14, 2018
Merged
shargon added a commit that referenced this pull request Sep 14, 2018
@shargon
Update Murmur3.cs
fcf3627 
Related to #376
@shargon shargon mentioned this pull request Sep 14, 2018
Closed
@shargon
Copy link
Copy Markdown
Member Author

shargon commented Sep 14, 2018

I think that all is fixed now, i close it

@shargon shargon closed this Sep 14, 2018
@shargon shargon deleted the fix-memory-access branch September 14, 2018 08:24
Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020
@Celia18305 @PeterLinX
Update getapplicationlog.md (neo-project#376)
1b2258a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erikzhang erikzhang Awaiting requested review from erikzhang

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@shargon @erikzhang