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

Check mstp_len before dereferencing #617

Closed
wants to merge 1 commit into from

Conversation

anarcat
Copy link

@anarcat anarcat commented Jul 20, 2017

During STP processing, we have a special handler for SPB (Shortest
Path Bridging) which parses a part of the packet for that specific
protocol. To find the length of that part of the packet (spb_len),
it uses the mstp_len variable to find where that value is
stored. Since mstp_len is unpacked from the packet earlier, it is a
user-controlled value and so, the pointer to sbp_len can also be
user-controlled.

We therefore need to check that we can extract it safely, using the
ND_TCHECK_16BITS macro, as per Code style and generic remarks in
the CONTRIBUTING document. The pointer taken to extract mstp_len
itself is checked, but, oddly, even though the comments say
sbp_len is validated as well, the macro call is missing.

This fixes the symptoms described by CVE-2017-11108 in my tests, but
someone with e deeper knowledge of the code should review this to
confirm it is the correct solution.

Closes: #616

During STP processing, we have a special handler for `SPB` (Shortest
Path Bridging) which parses a part of the packet for that specific
protocol. To find the length of that part of the packet (`spb_len`),
it uses the `mstp_len` variable to find where that value is
stored. Since `mstp_len` is unpacked from the packet earlier, it is a
user-controlled value and so, the pointer to `sbp_len` can also be
user-controlled.

We therefore need to check that we can extract it safely, using the
`ND_TCHECK_16BITS` macro, as per *Code style and generic remarks* in
the `CONTRIBUTING` document. The pointer taken to extract `mstp_len`
itself *is* checked, but, oddly, even though the comments say
`sbp_len` is validated as well, the macro call is missing.

This fixes the symptoms described by CVE-2017-11108 in my tests, but
someone with e deeper knowledge of the code should review this to
confirm it is the correct solution.

Closes: the-tcpdump-group#616
@anarcat
Copy link
Author

anarcat commented Jul 20, 2017

also note that there may be other branches of the stp code that are vulnerable to similar issues. i haven't audited the whole thing, just the part reported as vulnerable.

@infrastation
Copy link
Member

Commit d9e65de is a more complete solution to this problem, it is now available in tcpdump release 4.9.1. Thank you very much for the pull request, please send any future security reports to security@tcpdump.org.

@anarcat anarcat deleted the CVE-2017-11108 branch July 28, 2017 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

CVE-2017-11108 - DoS in print-stp.c
2 participants