Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix possible memory confusion in unsafe slice cast #204

Merged
merged 1 commit into from
Jun 3, 2020
Merged

Fix possible memory confusion in unsafe slice cast #204

merged 1 commit into from
Jun 3, 2020

Conversation

jlauinger
Copy link
Contributor

Description:

I found an incorrect cast from string to []byte in bytes_unsafe.go. The problem is that when reflect.SliceHeader is created as a composite literal (instead of deriving it from an actual slice by cast), then the Go garbage collector will not treat its Data field as a reference. If the GC runs just between creating the SliceHeader and casting it into the final, real []byte slice, then the underlying data might have been collected already, effectively making the returned []byte slice a dangling pointer.

This has a low probability to occur, but projects that import this library might still use it in a code path that gets executed a lot, thus increasing the probability to happen. Depending on the memory layout at the time of the GC run, this could potentially create an information leak vulnerability.

This PR changes the function to create the reflect.SliceHeader from an actual slice by first instantiating the return value.

Benchmark before change: n/a

Benchmark after change: n/a

For running benchmarks use:

go test -test.benchmem -bench JsonParser ./benchmark/ -benchtime 5s -v
# OR
make bench (runs inside docker)

Both failed for me, therefore I skipped running the benchmark, sorry about that!

@jlauinger jlauinger changed the title Fix possible memory confusion in unsafe slice cast fix: possible memory confusion in unsafe slice cast May 30, 2020
@jlauinger jlauinger changed the title fix: possible memory confusion in unsafe slice cast Fix possible memory confusion in unsafe slice cast May 30, 2020
@AllenX2018
Copy link
Collaborator

LGTM!

@AllenX2018 AllenX2018 merged commit 4eafe9b into buger:master Jun 3, 2020
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.

2 participants