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

GODRIVER-2869 Two protocol validations to reduce client denial of service risks #1291

Merged
merged 4 commits into from
Jun 22, 2023

Conversation

jentfoo
Copy link
Contributor

@jentfoo jentfoo commented Jun 9, 2023

GODRIVER-2869

Summary

This PR is to fix two possible conditions which could result in a potential denial of service of a client connected to a malicious MongoDB server.

  • Commit 1 - readLengthBytes requires the 4 bytes for the length to be included. Previously when reading a document from the wire this could result in a tight loop where an empty struct is appended to a slice repeatedly until the service runs out of memory (both CPU and memory consumption).
  • Commit 2 - Fix a large memory allocation condition with Snappy decompression if a large size is encoded in the Snappy compressed / encoded portion of the bytes.

Let me know if you have additional questions or if there is anything more I can do to help. Thank you!

jentfoo and others added 3 commits June 9, 2023 15:07
The length by definition needs to include the 4 bytes for the size.
If a size of zero is provided this can result in an infinite loop as providing the zero to `src[l:]` will result in the slice not being consumed.  Allowing the zero length to be read for infinity.
…tput

In addition tot he MongoDB protocol Snappy also has an encoded size.
If the Snappy payload is larger (or at least claims to be larger), the Snappy module will allocate a new byte slice to match the needed size.
This could result in memory exhaustion.
@qingyang-hu qingyang-hu changed the title Two protocol validations to reduce client denial of service risks GODRIVER-2869 Two protocol validations to reduce client denial of service risks Jun 12, 2023
@jentfoo
Copy link
Contributor Author

jentfoo commented Jun 21, 2023

@qingyang-hu and @matthewdale, is there anything further I can do to help with this PR?

Copy link
Collaborator

@matthewdale matthewdale left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! 👍

@qingyang-hu qingyang-hu merged commit a888dc6 into mongodb:master Jun 22, 2023
jentfoo added a commit to gravitational/teleport that referenced this pull request Jul 26, 2023
This updates the MongoDB driver to use a recent master commit in order to incorporate the fix: mongodb/mongo-go-driver#1291 (which has not been released to a tag yet)

In addition a fuzz seed was added to cover this condition.  After not finding more issues this also re-enables the MongoDB fuzzing in oss-fuzz.
github-merge-queue bot pushed a commit to gravitational/teleport that referenced this pull request Jul 27, 2023
This updates the MongoDB driver to use a recent master commit in order to incorporate the fix: mongodb/mongo-go-driver#1291 (which has not been released to a tag yet)

In addition a fuzz seed was added to cover this condition.  After not finding more issues this also re-enables the MongoDB fuzzing in oss-fuzz.
qingyang-hu added a commit that referenced this pull request Aug 1, 2023
…vice risks (#1291)

Co-authored-by: Alan Parra <[email protected]>
Co-authored-by: Qingyang Hu <[email protected]>
prestonvasquez pushed a commit to prestonvasquez/mongo-go-driver that referenced this pull request Aug 1, 2023
prestonvasquez pushed a commit to prestonvasquez/mongo-go-driver that referenced this pull request Aug 1, 2023
prestonvasquez pushed a commit to prestonvasquez/mongo-go-driver that referenced this pull request Aug 1, 2023
prestonvasquez pushed a commit to prestonvasquez/mongo-go-driver that referenced this pull request Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants