Skip to content
Permalink
Browse files Browse the repository at this point in the history
Use LZ4_decompress_safe instead of LZ4_uncompress
LZ4_uncompress is deprecated and has security problems.
  • Loading branch information
jgrahamc committed Jul 11, 2014
1 parent 2dcef6a commit 199f5f7
Showing 1 changed file with 6 additions and 7 deletions.
13 changes: 6 additions & 7 deletions lz4.go
Expand Up @@ -6,6 +6,7 @@ package lz4
import "C"

import (
"errors"
"fmt"
"unsafe"
)
Expand All @@ -25,14 +26,12 @@ func clen(s []byte) C.int {

// Uncompress with a known output size. len(out) should be equal to
// the length of the uncompressed out.
func Uncompress(in, out []byte) (err error) {
read := int(C.LZ4_uncompress(p(in), p(out), clen(out)))

if read != len(in) {
err = fmt.Errorf("uncompress read %d bytes should have read %d",
read, len(in))
func Uncompress(in, out []byte) (error) {
if int(C.LZ4_decompress_safe(p(in), p(out), clen(in), clen(out))) < 0 {
return errors.New("Malformed compression stream")
}
return

return nil
}

// CompressBound calculates the size of the output buffer needed by
Expand Down

4 comments on commit 199f5f7

@securitymouse
Copy link

Choose a reason for hiding this comment

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

This is technically incorrect. LZ4_decompress_safe was also proven exploitable in the same way that LZ4_uncompress variants is exploitable. LZ4_decompress_safe has no additional security protections in versions of LZ4 prior to r119. Versions of LZ4 >= r119 are still vulnerable on 64bit platforms.
http://blog.securitymouse.com/2014/07/the-lz4-two-hour-challenge.html

@Cyan4973
Copy link

Choose a reason for hiding this comment

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

Moving from a function which, by API definition, is deprecated and only supports correctly formed input (LZ4_uncompress),
to a function which, by API definition, also supports malicious input scenarios (LZ4_decompress_safe)
seems a beneficial evolution for a library which will likely need to work with untrusted (external) input in the near future.

Note : Context information : secMouse spent his last few weeks at creating multiple "vulnerability PoC" using LZ4_uncompress.

Versions of LZ4 >= r119 are still vulnerable on 64bit platforms.

If you have some elements to underline, you could, as countless developers did before, start by opening a new issue in the relevant public issue board and start a discussion there. You are singular in preferring to shout affirmations directly to the press and social medias without bothering to share your point with the relevant upstream organization.

@jgrahamc
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't seem helpful to me to have this argument here. Surely this is an argument about the LZ4 library that we are using here. We are now on the latest version r119. I'm not sure there's much more that this project can do to improve this situation.

If someone wants to make a suggestion I'm happy to hear it as a pull request, issue or on our HackerOne account: https://hackerone.com/

@securitymouse
Copy link

@securitymouse securitymouse commented on 199f5f7 Jul 14, 2014 via email

Choose a reason for hiding this comment

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

Please sign in to comment.