Skip to content
This repository has been archived by the owner on May 11, 2020. It is now read-only.

exec: expose memory size to Process #149

Merged
merged 1 commit into from
Jul 30, 2019
Merged

Conversation

laizy
Copy link
Member

@laizy laizy commented Jul 30, 2019

we current has some host function like:

func HostFunc(proc *exec.Process, keyPtr uint32, keylen uint32) {
	keybytes := make([]byte, keylen) 
	_, err := proc.ReadAt(keybytes, int64(keyPtr))
       ...
}

vicious wasm code can provide a large keylen to make the bytes allocation failed. Expose the current memory size can let us prevent this attack by checking keyPtr + keylen <= MemSize

Copy link
Contributor

@sbinet sbinet left a comment

Choose a reason for hiding this comment

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

nitpick then LGTM.

(thanks!)

exec/vm.go Outdated
@@ -531,6 +531,11 @@ func (proc *Process) WriteAt(p []byte, off int64) (int, error) {
return length, err
}

// MemSize returns the current allocated memory size in bytes
Copy link
Contributor

Choose a reason for hiding this comment

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

s/in bytes/in bytes./

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@codecov-io
Copy link

codecov-io commented Jul 30, 2019

Codecov Report

Merging #149 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #149      +/-   ##
==========================================
- Coverage   69.47%   69.44%   -0.03%     
==========================================
  Files          43       43              
  Lines        4921     4923       +2     
==========================================
  Hits         3419     3419              
- Misses       1217     1219       +2     
  Partials      285      285
Impacted Files Coverage Δ
exec/vm.go 83.27% <0%> (-0.6%) ⬇️

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 0864b86...c8ed6c5. Read the comment docs.

@sbinet sbinet merged commit 5a639f0 into go-interpreter:master Jul 30, 2019
@laizy laizy deleted the wagon branch July 30, 2019 12:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants