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

compress/flate: fix corrupted output #41477

Closed
wants to merge 5 commits into from

Conversation

klauspost
Copy link
Contributor

@klauspost klauspost commented Sep 18, 2020

The fastest compression mode can pick up a false match for every 2GB
of input data resulting in incorrectly decompressed data.

Since matches are allowed to be up to and including at maxMatchOffset
we must offset the buffer by an additional element to prevent the first
4 bytes to match after an out-of-reach value after shiftOffsets has
been called.

We offset by maxMatchOffset + 1 so offset 0 in the table will now
fail the if offset > maxMatchOffset in all cases.

Fixes #41420

Since matches are allowed to be up to and including at maxMatchOffset
we must offset the buffer by an additional element to prevent the first
4 bytes to match after an out-of-reach value after shiftOffsets has
been called.

We offset by `maxMatchOffset + 1` so offset 0 in the table will now
fail the `if offset > maxMatchOffset` in all cases.

Fixes golang#41420

Change-Id: Id6b054d9fd8adf2440d10490cb94962edac0bb02
@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Sep 18, 2020
@gopherbot
Copy link
Contributor

This PR (HEAD: 6343b56) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/255879 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ye Kuang:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Matthew Dempsky:

Patch Set 1: Run-TryBot+1 Trust+1

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 1:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=ab9b0164


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 1:

Build is still in progress...
This change failed on windows-386-2008:
See https://storage.googleapis.com/go-build-log/ab9b0164/windows-386-2008_f3781060.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 1: TryBot-Result-1

1 of 20 TryBots failed:
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/ab9b0164/windows-386-2008_f3781060.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

Change-Id: I1c1541f2c531d066aadfb78c4f1240da8de7770d
@gopherbot
Copy link
Contributor

This PR (HEAD: 9d082b9) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/255879 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

Change-Id: I614f77609038e99b72d456bdf37f86db1ee36449
@gopherbot
Copy link
Contributor

This PR (HEAD: 1157281) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/255879 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Klaus Post:

Patch Set 1:

(3 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Joe Tsai:

Patch Set 4: Code-Review+1

(4 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Joe Tsai:

Patch Set 4: Run-TryBot+1 Trust+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 4:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=50ca56a0


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 4: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

Change-Id: I093cff3f8e06142bbae0af67f50224be613a9ba1
@gopherbot
Copy link
Contributor

This PR (HEAD: 56c7485) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/255879 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Klaus Post:

Patch Set 4:

(4 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Takuto Ikuta:

Patch Set 5:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 1:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=ab9b0164


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 1:

Build is still in progress...
This change failed on windows-386-2008:
See https://storage.googleapis.com/go-build-log/ab9b0164/windows-386-2008_f3781060.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 1: TryBot-Result-1

1 of 20 TryBots failed:
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/ab9b0164/windows-386-2008_f3781060.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 4:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=50ca56a0


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 4: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Matthew Dempsky:

Patch Set 5:

(5 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

Change-Id: I1fa2dbc38b49b263788e666339a748761fe294f4
@gopherbot
Copy link
Contributor

This PR (HEAD: 50fabab) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/255879 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Klaus Post:

Patch Set 5:

(5 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Matthew Dempsky:

Patch Set 6: Code-Review+2


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Matthew Dempsky:

Patch Set 6: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 6:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=4165d899


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 6: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/255879.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Oct 19, 2020
The fastest compression mode can pick up a false match for every 2GB
of input data resulting in incorrectly decompressed data.

Since matches are allowed to be up to and including at maxMatchOffset
we must offset the buffer by an additional element to prevent the first
4 bytes to match after an out-of-reach value after shiftOffsets has
been called.

We offset by `maxMatchOffset + 1` so offset 0 in the table will now
fail the `if offset > maxMatchOffset` in all cases.

Fixes #41420

Change-Id: If1fbe01728e132b8a207e3f3f439edd832dcc710
GitHub-Last-Rev: 50fabab
GitHub-Pull-Request: #41477
Reviewed-on: https://go-review.googlesource.com/c/go/+/255879
Reviewed-by: Matthew Dempsky <[email protected]>
Run-TryBot: Matthew Dempsky <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Joe Tsai <[email protected]>
Trust: Matthew Dempsky <[email protected]>
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/255879 has been merged.

@gopherbot gopherbot closed this Oct 19, 2020
gopherbot pushed a commit that referenced this pull request Oct 29, 2020
The fastest compression mode can pick up a false match for every 2GB
of input data resulting in incorrectly decompressed data.

Since matches are allowed to be up to and including at maxMatchOffset
we must offset the buffer by an additional element to prevent the first
4 bytes to match after an out-of-reach value after shiftOffsets has
been called.

We offset by `maxMatchOffset + 1` so offset 0 in the table will now
fail the `if offset > maxMatchOffset` in all cases.

Updates #41420.
Fixes #41463.

Change-Id: If1fbe01728e132b8a207e3f3f439edd832dcc710
GitHub-Last-Rev: 50fabab
GitHub-Pull-Request: #41477
Reviewed-on: https://go-review.googlesource.com/c/go/+/255879
Reviewed-by: Matthew Dempsky <[email protected]>
Run-TryBot: Matthew Dempsky <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Joe Tsai <[email protected]>
Trust: Matthew Dempsky <[email protected]>
(cherry picked from commit ab541a0)
Reviewed-on: https://go-review.googlesource.com/c/go/+/266177
Run-TryBot: Dmitri Shuralyov <[email protected]>
Trust: Dmitri Shuralyov <[email protected]>
Reviewed-by: Joe Tsai <[email protected]>
lel-amri pushed a commit to lel-amri/zran-go that referenced this pull request Dec 16, 2024
The fastest compression mode can pick up a false match for every 2GB
of input data resulting in incorrectly decompressed data.

Since matches are allowed to be up to and including at maxMatchOffset
we must offset the buffer by an additional element to prevent the first
4 bytes to match after an out-of-reach value after shiftOffsets has
been called.

We offset by `maxMatchOffset + 1` so offset 0 in the table will now
fail the `if offset > maxMatchOffset` in all cases.

Updates #41420.
Fixes #41463.

Change-Id: If1fbe01728e132b8a207e3f3f439edd832dcc710
GitHub-Last-Rev: 50fabab0da874c37543b139459a810e12e83cee2
GitHub-Pull-Request: golang/go#41477
Reviewed-on: https://go-review.googlesource.com/c/go/+/255879
Reviewed-by: Matthew Dempsky <[email protected]>
Run-TryBot: Matthew Dempsky <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Joe Tsai <[email protected]>
Trust: Matthew Dempsky <[email protected]>
(cherry picked from commit ab541a0560408999ac65d12bec2a3057994eda38)
Reviewed-on: https://go-review.googlesource.com/c/go/+/266177
Run-TryBot: Dmitri Shuralyov <[email protected]>
Trust: Dmitri Shuralyov <[email protected]>
Reviewed-by: Joe Tsai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

compress/flate: deflatefast produces corrupted output
3 participants