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.

Sign up for GitHub

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

Jump to bottom

archive/zip: off-by-one writing directory64 records #14186

Closed
rsc opened this issue Feb 2, 2016 · 4 comments
Closed

archive/zip: off-by-one writing directory64 records #14186

rsc opened this issue Feb 2, 2016 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.8

Comments

@rsc
Copy link
Contributor

rsc commented Feb 2, 2016

archive/zip/writer.go says:

if records > uint16max || size > uint32max || offset > uint32max {

but each of these > should probably be >=.

See the similar bug fixed in https://go-review.googlesource.com/#/c/18317/ for writing data files of size uint32max.
@rsc rsc added this to the Go1.7 milestone Feb 2, 2016
@rsc rsc modified the milestones: Go1.8, Go1.7 May 16, 2016
@rsc rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 29, 2016
@rsc rsc modified the milestones: Go1.8Maybe, Go1.8 Sep 29, 2016
@quentinmit
Copy link
Contributor

records should definitely be >= since there is an additional end record. I think the size needs to account for directoryEndLen extra bytes. I don't see why offset needs to be >= however. So I propose this set of rules:

if records > uint16max-1 || size > uint32max-directoryEndLen || offset > uint32max

ping @rsc

@rsc
Copy link
Contributor Author

rsc commented Nov 9, 2016

In the zip format encoding, the ^0 value means "there is a bigger value encoded next". If you encode offset 0xFFFFFFFF meaning 0xFFFFFFFF, a decoder will see it as "there's a 64-bit value coming next". If the encoding does not include that 64-bit value, the decoder will get confused. See CL 18317 for an example of this with the file data size field.

@gopherbot
Copy link

CL https://golang.org/cl/33012 mentions this issue.

@rsc rsc modified the milestones: Go1.8, Go1.8Maybe Nov 11, 2016
@gopherbot
Copy link

CL https://golang.org/cl/33291 mentions this issue.

@golang golang locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.8
Development

No branches or pull requests
3 participants
@rsc @quentinmit @gopherbot