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

encoding/gob: harden against invalid input #2689

Closed
alberts opened this issue Jan 12, 2012 · 11 comments
Closed

encoding/gob: harden against invalid input #2689

alberts opened this issue Jan 12, 2012 · 11 comments
Assignees
Milestone

Comments

@alberts
Copy link
Contributor

@alberts alberts commented Jan 12, 2012

What steps will reproduce the problem?

gob decode invalid data

What is the expected output?

an error

What do you see instead?

lots of memory is allocated for slices

Please provide any additional information below.

https://groups.google.com/group/golang-nuts/browse_thread/thread/e3c85563eb0b31aa

https://groups.google.com/group/golang-nuts/browse_thread/thread/f661b5b3b0b2ec15
@robpike
Copy link
Contributor

@robpike robpike commented Jan 13, 2012

Comment 1:

Labels changed: added priority-go1, removed priority-triage.

@robpike
Copy link
Contributor

@robpike robpike commented Jan 13, 2012

Comment 2:

Status changed to Accepted.

@robpike
Copy link
Contributor

@robpike robpike commented Jan 13, 2012

Comment 3:

Owner changed to builder@golang.org.

@dsymonds
Copy link
Member

@dsymonds dsymonds commented Jan 20, 2012

Comment 4:

Owner changed to @dsymonds.

@robpike
Copy link
Contributor

@robpike robpike commented Jan 22, 2012

Comment 5:

Other than protection from very large buffer sizes, do you have an actual bug to report?
Gob has protection against "invalid data", although it's likely there are cases that
slip through the cracks. Please be more specific.
@robpike
Copy link
Contributor

@robpike robpike commented Jan 22, 2012

Comment 6:

Also, I double-checked and the code has some protection against huge message sizes. With
a specific example it might be possible to harden that further.
It would also be easy to allow programmer control of the upper limit of a message size
or slice, but I am reluctant to do that because of the clumsiness and brittleness that
would result.
@robpike
Copy link
Contributor

@robpike robpike commented Jan 22, 2012

Comment 7:

I dropped the max message from 2^31 to 2^30. Moreover, the old code was probably broken
because at 2^31 it could overflow. Please sync, run whatever tests you have, and report
back.
@dsymonds
Copy link
Member

@dsymonds dsymonds commented Jan 22, 2012

Comment 8:

One obvious thing I thought of is that if a slice or string (or similar) is encoded with
a very large length, and there's not that many bytes in the stream, then there's no
point in allocating memory for what is obviously an invalid message. However, gob is a
stream encoding, so that doesn't make so much sense.

Status changed to WaitingForReply.

@alberts
Copy link
Contributor Author

@alberts alberts commented Jan 25, 2012

Comment 10:

Here's a start with a framework for adding more test types. Kinds of fuzzing could also
still be expanded. It's turned up two bugs though:
gotest -test.run=TestFuzzBug1
panic: runtime error: makeslice: len out of range [recovered]
        panic: interface conversion: interface is runtime.errorString, not gob.gobError
gotest -test.run=TestFuzzBug2
an infinite loop or something that takes a really long time.
It's tricky to build more tests at present because I frequently hit one of these two.

Attachments:

  1. fuzz_test.go (1348 bytes)
@dsymonds
Copy link
Member

@dsymonds dsymonds commented Feb 3, 2012

Comment 11:

Status changed to Started.

@dsymonds
Copy link
Member

@dsymonds dsymonds commented Feb 6, 2012

Comment 12:

This issue was closed by revision 9440d82.

Status changed to Fixed.

@alberts alberts added fixed labels Feb 6, 2012
@rsc rsc added this to the Go1 milestone Apr 10, 2015
@rsc rsc removed the priority-go1 label Apr 10, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.