Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
138 changes: 72 additions & 66 deletions cves/kernel/CVE-2016-9754.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: '2016-05-13'
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: |
The ring_buffer_resize function used for converting a buffer for use in a userspace
in the profiling subsystem mishandles certain integer calculations. This
miscalculation is capable of causing an overflow during integer rounding. This
allows users to gain unintended privileges by changing the buffer size by
writing to the /sys/kernel/debug/tracing/buffer_size_kb file.
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,7 @@ bugs_instructions: |
* Mentioned in mailing list discussions
* References from NVD entry
* Various other places
bugs: []
bugs: [118001, 112571, 106031, 114201, 116651]
fixes_instructions: |
Please put the commit hash in "commit" below.

Expand All @@ -84,14 +89,11 @@ fixes_instructions: |

Place any notes you would like to make in the notes field.
fixes:
- commit:
note:
- commit:
note:
- commit: 0b8eecc14410eaa197809f66f4de8fa85c2721ed
note: 'Found in kernel.org changelog: "ring-buffer: Use long for nr_pages to avoid overflow failures"'
- commit: 59643d1535eb220668692a5359de22545af579f6
note: |
Taken from NVD references list with Git commit. If you are
curating, please fact-check that this commit fixes the vulnerability and replace this comment with 'Manually confirmed'
Manually Confirmed
vcc_instructions: |
The vulnerability-contributing commits.

Expand All @@ -106,17 +108,17 @@ vcc_instructions: |
Place any notes you would like to make in the notes field.
vccs:
- commit: 7a8e76a3829f1067b70f715771ff88baf2fbf3c3
note: Discovered automatically by archeogit.
note: 'Discovered automatically by archeogit. Introduces the ring buffer.'
- commit: 83f40318dab00e3298a1f6d0b12ac025e84e478d
Copy link

Choose a reason for hiding this comment

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

This bug was in the system for 4 years before being detected. I had a similar CVE with very old code containing a bug. Who knows how many undiscovered bugs are tucked away in large systems. Very interesting.

note: Discovered automatically by archeogit.
note: 'Discovered automatically by archeogit. Adds the capability to remove pages from a ring buffer without destroying any existing data in it.'
upvotes_instructions: |

Choose a reason for hiding this comment

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

1 upvote

For the first round, ignore this upvotes number.

For the second round of reviewing, you will be giving a certain amount of
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: 8
unit_tested:
question: |
Were automated unit tests involved in this vulnerability?
Expand All @@ -131,10 +133,10 @@ 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: 'No unit tests were added when the ring buffer feature was added'
fix: false
fix_answer: 'No unit tests were added in the fixes'
discovered:
question: |
How was this vulnerability discovered?
Expand All @@ -149,10 +151,10 @@ discovered:

If there is no evidence as to how this vulnerability was found, then please
explain where you looked.
answer:
automated:
contest:
developer:
answer: 'The issue was reported by a Red Hat employee'
automated: false
contest: false
developer: false
Comment on lines +154 to +157
Copy link

Choose a reason for hiding this comment

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

Was the method of discovery mentioned by the employee?

Copy link
Author

Choose a reason for hiding this comment

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

It was not. I assume it was a manual find.

autodiscoverable:
instructions: |
Is it plausible that a fully automated tool could have discovered
Expand All @@ -169,8 +171,8 @@ autodiscoverable:

The answer field should be boolean. In answer_note, please explain
why you come to that conclusion.
note:
answer:
note: 'Integer overflows can be found by automated tools.'
answer: true
specification:
instructions: |
Is there mention of a violation of a specification? For example, the POSIX
Expand All @@ -186,8 +188,8 @@ specification:

The answer field should be boolean. In answer_note, please explain
why you come to that conclusion.
note:
answer:
note: 'No specifications are mentioned in the commit, bug report, or mail chain'
answer: false
subsystem:
question: |
What subsystems was the mistake in? These are WITHIN linux kernel
Expand Down Expand Up @@ -221,7 +223,7 @@ subsystem:
e.g.
name: ["subsystemA", "subsystemB"] # ok
name: subsystemA # also ok
name:
name: profiling
note:
interesting_commits:
question: |
Expand All @@ -237,10 +239,8 @@ interesting_commits:
* Other commits that fixed a similar issue as this vulnerability
* Anything else you find interesting.
commits:
- commit:
note:
- commit:
note:
- commit: 9b94a8fba501f38368aef6ac1b30e7335252a220
note: This seems to be an attempt at fixing the integer overflow. However it didnt cover every cause, so overflow was still possible.
i18n:
question: |
Was the feature impacted by this vulnerability about internationalization
Expand All @@ -253,8 +253,8 @@ 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: 'The issue was regarding byte buffers. Characters were not involved.'
sandbox:
question: |
Did this vulnerability violate a sandboxing feature that the system
Expand All @@ -268,8 +268,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: 'The feature that has the issue is a method of storing paged data. It is not a sandbox feature.'
ipc:
question: |
Did the feature that this vulnerability affected use inter-process
Expand All @@ -280,8 +280,8 @@ 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: true
note: 'The exploit involves user input to modify a file controlling buffer size.'

Choose a reason for hiding this comment

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

How did you come to this conclusion?

This seems to be a vulnerability in a kernel data structure.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, but the exploit noted in the fix commit involves editing a file that controls the buffer size used in the ring buffer, /sys/kernel/debug/tracing/buffer_size_kb. This is also noted in the vulnerability description on NVD.

discussion:
question: |
Was there any discussion surrounding this?
Expand All @@ -307,9 +307,9 @@ 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: false
any_discussion: false
note: No bug discussion, and the mail chain is just a series of concurrent fixes.
vouch:
question: |
Was there any part of the fix that involved one person vouching for
Expand All @@ -322,8 +322,8 @@ 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: 'The commit has an acknowledgement reading "Signed-off-by: Steven Rostedt <[email protected]>"'
stacktrace:
question: |
Are there any stacktraces in the bug reports?
Expand All @@ -337,9 +337,9 @@ 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: The commit and mail chain do not have a stacktrace, only instructions for reproduction and likely cause.
forgotten_check:
question: |
Does the fix for the vulnerability involve adding a forgotten check?
Expand All @@ -358,8 +358,8 @@ 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: 'The fix involved changing incorrect calculations. No checks were added.'
order_of_operations:
question: |
Does the fix for the vulnerability involve correcting an order of
Expand All @@ -371,8 +371,8 @@ 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: true
note: The order of rounding up the buffer size and multiplying the buffer size by the page count has been reversed.
lessons:
question: |
Are there any common lessons we have learned from class that apply to this
Expand All @@ -389,38 +389,38 @@ 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:
applies: false
note:

Choose a reason for hiding this comment

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

Consider adding a note for why it's false. Other documents I've reviewed included this, so it might be good to have it.

least_privilege:
applies:
applies: false
note:
frameworks_are_optional:
applies:
applies: false
note:
native_wrappers:
applies:
applies: false
note:
distrust_input:
applies:
note:
applies: true
note: The functionality previously trusted the buffer sizes from a file that could be modified by a user.
security_by_obscurity:
applies:
applies: false
note:
serial_killer:
applies:
applies: false
note:
environment_variables:
applies:
applies: false
note:
secure_by_default:
applies:
applies: false
note:
yagni:
applies:
applies: false
note:
complex_inputs:
applies:
note:
applies: true
note: The functionality lacks boundary checks for the integer that controls buffer size.
mistakes:
question: |
In your opinion, after all of this research, what mistakes were made that
Expand Down Expand Up @@ -450,7 +450,14 @@ mistakes:

Write a thoughtful entry here that people in the software engineering
industry would find interesting.
answer:
answer: |
This was likely a planning error. The issue was caused by a mixup in algebraic logic.
I suspect this was simply a lack of oversight and concept testing when designing said
logic. The CWE recommends that a behavior that is seen as 'out-of-bounds' is strictly
defined. This is not seen in the code itself, but is possibly present in some planning
documentation. There is also no check to make sure the buffer is in bounds. The other
mitigations, however, about clearly understanding the behavior of the signed integers
that are used, are met.
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 @@ -465,15 +472,14 @@ CWE_instructions: |
apply here, then place them in an array like this
CWE: ["123", "456"] # this is ok
CWE: [123, 456] # also ok
CWE: 123 # also ok
CWE: true23 # also ok
CWE:
- 190
CWE_note: |
CWE as registered in the NVD. If you are curating, check that this
is correct and replace this comment with "Manually confirmed".
Manually Confirmed
nickname_instructions: |
A catchy name for this vulnerability that would draw attention it.
If the report mentions a nickname, use that.
Must be under 30 characters. Optional.
nickname:
nickname: Ring Buffer Overflow
CVSS: CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H
Loading