Skip to content
New issue

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

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

Already on GitHub? # to your account

Deflate decompression hangs on trailing garbage #176

Closed
vojtarylko opened this issue Sep 14, 2022 · 5 comments
Closed

Deflate decompression hangs on trailing garbage #176

vojtarylko opened this issue Sep 14, 2022 · 5 comments

Comments

@vojtarylko
Copy link

I tried to extend HTTPRequestDecompressorTest with this test case:

    func testDecompressionTrailingData() throws {
        // Valid compressed data with some trailing garbage
        let compressed = ByteBuffer(bytes: [120, 156, 99, 0, 0, 0, 1, 0, 1] + [1, 2, 3])

        let channel = EmbeddedChannel()
        try channel.pipeline.addHandler(NIOHTTPRequestDecompressor(limit: .none)).wait()
        let headers = HTTPHeaders([("Content-Encoding", "deflate"), ("Content-Length", "\(compressed.readableBytes)")])
        try channel.writeInbound(HTTPServerRequestPart.head(.init(version: .init(major: 1, minor: 1), method: .POST, uri: "https://nio.swift.org/test", headers: headers)))

        try channel.writeInbound(HTTPServerRequestPart.body(compressed))
    }

and when tried to run it, the test hangs forever.

If confirmed, this is security issue which can cause DoS.

Lukasa added a commit to Lukasa/swift-nio-extras that referenced this issue Sep 15, 2022
Motivation

Currently we don't confirm that the decompression has completed
successfully. This means that we can incorrectly spin forever attempting
to decompress past the end of a message, and that we can fail to notice
that a message is truncated. Neither of these is good.

Modifications

Propagate the message zlib gives us as to whether or not decompression
is done, and keep track of it.
Add some tests written by @vojtarylko to validate the behaviour.

Result

Correctly police the bounds of the messages.
Resolves apple#175 and apple#176.
Lukasa added a commit to Lukasa/swift-nio-extras that referenced this issue Sep 15, 2022
Motivation

Currently we don't confirm that the decompression has completed
successfully. This means that we can incorrectly spin forever attempting
to decompress past the end of a message, and that we can fail to notice
that a message is truncated. Neither of these is good.

Modifications

Propagate the message zlib gives us as to whether or not decompression
is done, and keep track of it.
Add some tests written by @vojtarylko to validate the behaviour.

Result

Correctly police the bounds of the messages.
Resolves apple#175 and apple#176.
@Lukasa
Copy link
Contributor

Lukasa commented Sep 15, 2022

Great catch, resolved by #177. I'll try to get a fix out ASAP. I'll also retroactively apply for a CVE and publish a notification, but as this was publicly reported I'd like to get the release out first. Thanks so much for reporting!

Lukasa added a commit that referenced this issue Sep 16, 2022
Motivation

Currently we don't confirm that the decompression has completed
successfully. This means that we can incorrectly spin forever attempting
to decompress past the end of a message, and that we can fail to notice
that a message is truncated. Neither of these is good.

Modifications

Propagate the message zlib gives us as to whether or not decompression
is done, and keep track of it.
Add some tests written by @vojtarylko to validate the behaviour.

Result

Correctly police the bounds of the messages.
Resolves #175 and #176.
Lukasa added a commit to Lukasa/swift-nio-extras that referenced this issue Sep 16, 2022
Motivation

Currently we don't confirm that the decompression has completed
successfully. This means that we can incorrectly spin forever attempting
to decompress past the end of a message, and that we can fail to notice
that a message is truncated. Neither of these is good.

Modifications

Propagate the message zlib gives us as to whether or not decompression
is done, and keep track of it.
Add some tests written by @vojtarylko to validate the behaviour.

Result

Correctly police the bounds of the messages.
Resolves apple#175 and apple#176.

(cherry picked from commit 6c84d24)
Lukasa added a commit to Lukasa/swift-nio-extras that referenced this issue Sep 16, 2022
Motivation

Currently we don't confirm that the decompression has completed
successfully. This means that we can incorrectly spin forever attempting
to decompress past the end of a message, and that we can fail to notice
that a message is truncated. Neither of these is good.

Modifications

Propagate the message zlib gives us as to whether or not decompression
is done, and keep track of it.
Add some tests written by @vojtarylko to validate the behaviour.

Result

Correctly police the bounds of the messages.
Resolves apple#175 and apple#176.

(cherry picked from commit 6c84d24)
@Lukasa
Copy link
Contributor

Lukasa commented Sep 16, 2022

Fix is out.

@Lukasa Lukasa closed this as completed Sep 16, 2022
@vojtarylko
Copy link
Author

@Lukasa Thanks for fast fix! Was CVE published already?

@Lukasa
Copy link
Contributor

Lukasa commented Sep 19, 2022

Not yet, I’ll update this issue when it is.

@Lukasa
Copy link
Contributor

Lukasa commented Sep 21, 2022

We've assigned this CVE-2022-3252.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants