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

CVE-2017-18549 and CVE-2018-13406 #153

Open
wants to merge 17 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 91 additions & 42 deletions cves/kernel/CVE-2017-18549.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ curated_instructions: |
This will enable additional editorial checks on this file to make sure you
fill everything out properly. If you are a student, we cannot accept your work
as finished unless curated is properly updated.
curation_level: 0
curation_level: 2
reported_instructions: |
What date was the vulnerability reported to the security team? Look at the
security bulletins and bug reports. It is not necessarily the same day that
the CVE was created. Leave blank if no date is given.

Please enter your date in YYYY-MM-DD format.
reported_date:
reported_date: 2017-06-23
announced_instructions: |
Was there a date that this vulnerability was announced to the world? You can
find this in changelogs, blogs, bug reports, or perhaps the CVE date.
Expand Down Expand Up @@ -55,7 +55,12 @@ description_instructions: |

Your target audience is people just like you before you took any course in
security
description:
description: |
RAM is (virtually) segregated into two parts, the Kernelspace (where privileged kernel programs and drivers run)
and the Userspace (where regular user software runs). In C, an uninitiated variable points to the address allocated in the Kernelspace,
and the variable holds whatever was in there from before (which could be sensitive information).
In this vulnerability this uninitialized variable's data is sent from the Kernelspace into the Userspace.
AliJafriRIT marked this conversation as resolved.
Show resolved Hide resolved

bounty_instructions: |
If you came across any indications that a bounty was paid out for this
vulnerability, fill it out here. Or correct it if the information already here
Expand All @@ -75,7 +80,8 @@ bugs_instructions: |
* Mentioned in mailing list discussions
* References from NVD entry
* Various other places
bugs: []
bugs:
- "https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1700077"
fixes_instructions: |
Please put the commit hash in "commit" below.

Expand Down Expand Up @@ -116,7 +122,7 @@ upvotes_instructions: |
upvotes to each vulnerability you see. Your peers will tell you how
interesting they think this vulnerability is, and you'll add that to the
upvotes score on your branch.
upvotes:
upvotes: 5
unit_tested:
question: |
Were automated unit tests involved in this vulnerability?
Expand All @@ -131,10 +137,16 @@ unit_tested:

For the fix_answer below, check if the fix for the vulnerability involves
adding or improving an automated test to ensure this doesn't happen again.
code:
code_answer:
fix:
fix_answer:
code: false
code_answer: |
Generally the linux kernel is tested using the KUnit test suite.
However, I couldn't find any unit tests for the commctrl.c file. Interestingly, in the same folder,
there is a "TODO" file which says "Testing, More Testing, I/O Size Increase".
This suggests that unit tests were not conducted fro this file.
fix: false
fix_answer: |
Unit tests (most likely) would not help detect such vulnerabilities since if the variable is uninitalized,
it still points to a value (in C). If it were null (like in most other languages), then unit tests could have been used.
discovered:
question: |
How was this vulnerability discovered?
Expand All @@ -149,10 +161,16 @@ discovered:

If there is no evidence as to how this vulnerability was found, then please
explain where you looked.
answer:
automated:
contest:
developer:
answer: |
"I was unable to find how the vulnerability was discovered. I
looked at the vulnerability on the NIST NVD database and also looked for it in
the commit. I even checked the linux kernel hypermail, but my search led me
nowhere. I do know that it was discovered by Seth Forshee. There is no
indication of it being found by automated means or its discovery being
associated with a competition. "
automated: false
contest: false
developer: false
autodiscoverable:
instructions: |
Is it plausible that a fully automated tool could have discovered
Expand All @@ -169,8 +187,12 @@ autodiscoverable:

The answer field should be boolean. In answer_note, please explain
why you come to that conclusion.
note:
answer:
note: |
It is unlikely that it was discovered through automated means or a fuzzer.
This is an issue in the code. It pertains to an uninitialized variable resulting in a memory leak to the userspace.
There is no compiler warning or error message indicating that there was a memory leak,
which makes it difficult to automated tools to detect.
answer: false
AliJafriRIT marked this conversation as resolved.
Show resolved Hide resolved
specification:
instructions: |
Is there mention of a violation of a specification? For example, the POSIX
Expand All @@ -186,8 +208,9 @@ specification:

The answer field should be boolean. In answer_note, please explain
why you come to that conclusion.
note:
answer:
note: |
"No indication of any specification violations. I checked commit, the mailing lists and bug reports"
answer: false
subsystem:
question: |
What subsystems was the mistake in? These are WITHIN linux kernel
Expand Down Expand Up @@ -221,8 +244,9 @@ subsystem:
e.g.
name: ["subsystemA", "subsystemB"] # ok
name: subsystemA # also ok
name:
note:
name: drivers
note: |
The file commctrl.c is in the 'drivers' directory, which means it pertains to the drivers subsystem.
interesting_commits:
question: |
Are there any interesting commits between your VCC(s) and fix(es)?
Expand All @@ -241,6 +265,13 @@ interesting_commits:
note:
- commit:
note:
- commit: 5cc973f09e21b5a2f746307641879bc9f1da623b
note: |
This was this commit right before the error was fixed. This also involves a memory leak into the userspace.
Here, a variable was uninitialized and its contents were being written to the userspace. Based on the nature of the error,
it seems that it is quite common. The issue is that an uninitialized variable will always contain a garbage value from the kernel stack.
This 'garbage' can contain important information which could be used maliciously if it is passed to the userspace,
AliJafriRIT marked this conversation as resolved.
Show resolved Hide resolved
where it is more accessible to users and attackers.
i18n:
question: |
Was the feature impacted by this vulnerability about internationalization
Expand All @@ -253,8 +284,10 @@ i18n:
Answer should be true or false
Write a note about how you came to the conclusions you did, regardless of
what your answer was.
answer:
note:
answer: false
note: |
This vulnerability is about potentially sensitive information being passed from the kernelspace to the userspace.
It doesn't particularly involve internationalization.
sandbox:
question: |
Did this vulnerability violate a sandboxing feature that the system
Expand All @@ -268,8 +301,8 @@ sandbox:
Answer should be true or false
Write a note about how you came to the conclusions you did, regardless of
what your answer was.
answer:
note:
answer: false
note: Not a sandboxing vulnerability, since it pertains to a memory leak to the userspace, which is not access control realted.
ipc:
question: |
Did the feature that this vulnerability affected use inter-process
Expand All @@ -280,8 +313,10 @@ ipc:
Answer must be true or false.
Write a note about how you came to the conclusions you did, regardless of
what your answer was.
answer:
note:
answer: false
note: |
The vulnerability involves directly writing to the userspace in RAM.
There is no inter process communication that was affected by this.
discussion:
question: |
Was there any discussion surrounding this?
Expand All @@ -307,9 +342,11 @@ discussion:

Put any links to disagreements you found in the notes section, or any other
comment you want to make.
discussed_as_security:
any_discussion:
note:
discussed_as_security: true
any_discussion: true
note: |
They discussed about how zeroing out the variable would prevent the issue from occurring.
Link: https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2017-18549
vouch:
question: |
Was there any part of the fix that involved one person vouching for
Expand All @@ -322,8 +359,9 @@ vouch:

Answer must be true or false.
Write a note about how you came to the conclusions you did, regardless of what your answer was.
answer:
note:
answer: true
note: |
Commit Signed off by Martin K. Petersen
stacktrace:
question: |
Are there any stacktraces in the bug reports?
Expand All @@ -337,9 +375,10 @@ stacktrace:
Answer must be true or false.
Write a note about how you came to the conclusions you did, regardless of
what your answer was.
any_stacktraces:
stacktrace_with_fix:
note:
any_stacktraces: false
stacktrace_with_fix: false
note: |
Copy link
Contributor

Choose a reason for hiding this comment

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

C still has stacktraces! But I'm not surprised that there wasn't one here.

None, because the Linux Kernel is written in C.
forgotten_check:
question: |
Does the fix for the vulnerability involve adding a forgotten check?
Expand All @@ -358,8 +397,10 @@ forgotten_check:
Answer must be true or false.
Write a note about how you came to the conclusions you did, regardless of
what your answer was.
answer:
note:
answer: false
note: |
Checks would not particularly be helpful as the uninitialized variable still contains data from the kernel stack.
Traditionally we would check if the variable is 'null' to see if its uninitialized, but that doesn't apply here.
order_of_operations:
question: |
Does the fix for the vulnerability involve correcting an order of
Expand All @@ -371,8 +412,9 @@ order_of_operations:
Answer must be true or false.
Write a note about how you came to the conclusions you did, regardless of
what your answer was.
answer:
note:
answer: false
note: |
The fix involves zeroing the variable out, if it is uninitiated.
lessons:
question: |
Are there any common lessons we have learned from class that apply to this
Expand All @@ -389,8 +431,12 @@ lessons:
If you think of another lesson we covered in class that applies here, feel
free to give it a small name and add one in the same format as these.
defense_in_depth:
applies:
note:
applies: true
note: |
Even if sensitive data from the kernelspace is leaked into the userspace,
a regular user wouldn't likely come across it or obtain it through a program,
only someone with malicious intent would do such a thing.
However, preventing such leakes in the first place would eliminate this issue altogether.
least_privilege:
applies:
note:
Expand Down Expand Up @@ -450,7 +496,10 @@ mistakes:

Write a thoughtful entry here that people in the software engineering
industry would find interesting.
answer:
answer: |
I thing the mistake made here would be categorized as a lapse,
since there the developer forgot to zero out the memory of the
uninitialized variable before its passed on to the userspace.
CWE_instructions: |
Please go to http://cwe.mitre.org and find the most specific, appropriate CWE
entry that describes your vulnerability. We recommend going to
Expand All @@ -475,5 +524,5 @@ nickname_instructions: |
A catchy name for this vulnerability that would draw attention it.
AliJafriRIT marked this conversation as resolved.
Show resolved Hide resolved
If the report mentions a nickname, use that.
Must be under 30 characters. Optional.
nickname:
CVSS:
nickname: Data leak to Userspace
CVSS: CVSS:3.0/AV:L/AC:L/PR:L/UI:N/S:U/C:H/I:N/A:N
Loading