CVE-2016-9754 and CVE-2022-0516 #225

evstod · 2023-11-10T19:33:28Z

No description provided.

andymeneely · 2023-11-11T14:30:04Z

Looks like this needs to be worked on. Keep going!

mineo333

Good work overall. Just a few comments.

mineo333 · 2023-11-12T06:56:57Z

cves/kernel/CVE-2016-9754.yml

@@ -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: 'Buffer overflows can be found by automated tools.'


This is not a buffer overflow, but rather an integer overflow - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=59643d1535eb220668692a5359de22545af579f6

This, under certain situations, can still be found by certain automated testers such as syzkaller

You're right. The integer in question controls the buffer size for the ring buffer, which caused me to misidentify the issue. Fixed the note.

cves/kernel/CVE-2016-9754.yml

mineo333 · 2023-11-12T07:00:54Z

cves/kernel/CVE-2016-9754.yml

-  - commit:
-    note:
+  - commit: 9b94a8fba501f38368aef6ac1b30e7335252a220
+    note: This seems to be an attempt at fixing the buffer overflow. Howvwer it didnt cover every cause, so overflow was still possible.


Same comment as above. This is an integer overflow.

You're right. The integer in question controls the buffer size for the ring buffer, which caused me to misidentify the issue. Fixed the note.

mineo333 · 2023-11-12T07:03:27Z

cves/kernel/CVE-2016-9754.yml

-  answer:
-  note:
+  answer: true
+  note: 'The exploit involves user input to modify a file controlling buffer size.'


How did you come to this conclusion?

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

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.

mineo333 · 2023-11-12T07:08:11Z

cves/kernel/CVE-2016-9754.yml

-    applies:
-    note:
+    applies: true
+    note: The functionality lacks boundary checks for the buffer.


There does not seem to be a buffer involved here.

I meant the integer that controls the buffer size. Fixed to clarify that better.

cves/kernel/CVE-2022-0516.yml

mineo333 · 2023-11-12T07:18:29Z

cves/kernel/CVE-2022-0516.yml

@@ -114,7 +115,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: false


mineo333 · 2023-11-12T07:18:57Z

cves/kernel/CVE-2016-9754.yml

 - commit: 83f40318dab00e3298a1f6d0b12ac025e84e478d
-  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: |


Co-authored-by: Sharad Khanna <[email protected]>

While the vuln regards an int that controls a buffer size, it is not a buffer overflow

This reverts commit afd6a55.

JoeWesn · 2023-11-15T01:12:34Z

cves/kernel/CVE-2016-9754.yml

+  answer: 'The issue was reported by a Red Hat employee'
+  automated: false
+  contest: false
+  developer: false


Was the method of discovery mentioned by the employee?

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

JoeWesn · 2023-11-15T01:19:20Z

cves/kernel/CVE-2016-9754.yml

 upvotes_instructions: |
  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: 1


JoeWesn · 2023-11-15T01:28:22Z

cves/kernel/CVE-2016-9754.yml

@@ -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


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.

JoeWesn · 2023-11-15T01:47:41Z

cves/kernel/CVE-2022-0516.yml

+  - commit: a43b80b782c9f56b3bcc2e5e51261dc3980839ec
+    note: | 
+      There is 2 years worth of work on this subsystem and even file from the origin
+      to the fix, but none of them really did anything with the Guest users functionality
+      except this one. This one adds a global for debugging, KVM_GUESTDBG_VALID_MASK.
+      Though it has no effect on the specific function of the bug, it is interesting that
+      permissions debugging systems were made in the bug's subsystem, and yet the bug
+      remained elusive.


Do you think they were aware there might have been a bug but didn't know the exact location and couldn't replicate it? Or was this preemptive yet still didn't locate it. Either way its unfortunate they walked right over it without seeing the bug.

JoeWesn · 2023-11-15T01:48:25Z

cves/kernel/CVE-2022-0516.yml

@@ -114,7 +115,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: 2


JoeWesn · 2023-11-15T01:52:37Z

cves/kernel/CVE-2022-0516.yml

+  answer: |
+    This is likely a simple lapse. The function used for making the check on the virtual cpu was created
+    and intnded to be used, but the check was never placed in along with the rest. The fix was simply adding the check in.
+    CWE 280 recommends using "safe areas" surrounded by trust boundaries in the design of the subsystem. These were assumably 
+    already in place given the specificness of the other checks in the permissions functionality. The CWE also recommends always
+    checking that a resource was properlly and successfully accessed. This is now followed since such a check was added.


Was the function to make the check used anywhere after its implementation?

I'm not sure. It was not used anywhere in the subsystem at least.

Patrick-Sorensen

Great work overall, and definitely an interesting vulnerability. Just added a couple minor suggestions to help with consistency and readability, looks like the majority of your information is laid out quite well otherwise though!

Also please add 2 upvotes to CVE-2022-0516

I found it interesting how long it took them to finally fix the issue, even after they knew they had a bug in their system

Patrick-Sorensen · 2023-11-15T04:19:20Z

cves/kernel/CVE-2022-0516.yml

@@ -105,7 +106,7 @@ vcc_instructions: |

  Place any notes you would like to make in the notes field.
 vccs:
- commit: 19e1227768863a1469797c13ef8fea1af7beac2c
+- commit: '19e1227768863a1469797c13ef8fea1af7beac2c'


The quotes are unnecessary here since it's a hash value. Not sure if it makes a difference but they can be removed both here and for the hash on line 91

Patrick-Sorensen · 2023-11-15T04:21:19Z

cves/kernel/CVE-2022-0516.yml

+  code: false
+  code_answer: 'There were no surrounding tests. Since this subsystem surrounds permissions. Unit tests handling this type of defect would be difficult.'
+  fix: false
+  fix_answer: 'No tests were added. Since this subsystem surrounds permissions. Unit tests handling this type of defect would be difficult.'


Could be refactored into paragraph format just to stay under the 80 char line limit. Same can be said for lines 320, 334, 350, etc.

Patrick-Sorensen · 2023-11-15T04:34:20Z

cves/kernel/CVE-2022-0516.yml

+    and intnded to be used, but the check was never placed in along with the rest. The fix was simply adding the check in.
+    CWE 280 recommends using "safe areas" surrounded by trust boundaries in the design of the subsystem. These were assumably 
+    already in place given the specificness of the other checks in the permissions functionality. The CWE also recommends always
+    checking that a resource was properlly and successfully accessed. This is now followed since such a check was added.


Just a couple grammatical updates here as well that could help with readability, intended and properly

karisanno11 · 2023-11-15T20:39:15Z

cves/kernel/CVE-2016-9754.yml

+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 
+  misculaculation is capable of causing an overflow during ineger rounding. This 


There are some typos here:
"misculaculation" -> "miscalculation"
"ineger rounding" -> "integer rounding"

karisanno11 · 2023-11-15T20:39:49Z

cves/kernel/CVE-2016-9754.yml

+  The ring_buffer_resize function used for converting a buffer for use in a userspace 
+  in the profiling subsystem mishandles certain integer calculations. This 
+  misculaculation is capable of causing an overflow during ineger rounding. This 
+  allows users to gain unintended privileges by changing the buffer suze by 


Typo:
"buffer suze" -> "buffer size"

karisanno11 · 2023-11-15T20:46:22Z

cves/kernel/CVE-2016-9754.yml

-  note:
+  any_stacktraces: false
+  stacktrace_with_fix: false
+  note: The commit and mail chain do not have a stacktrace, only instructions for repriduction and likely cause.


Typo: "repriduction" -> "reproduction"

karisanno11 · 2023-11-15T20:47:25Z

cves/kernel/CVE-2016-9754.yml

+  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 bahvior that is seen as 'out-of-bounds' is strictly 


Typo:
"bahvior" -> "behavior"

karisanno11 · 2023-11-15T20:48:43Z

cves/kernel/CVE-2016-9754.yml

+    logic. The CWE recommends that a bahvior 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 learly understanding the behavior of the signed integers 


"learly understanding"? Maybe it was supposed to be "clearly understanding"

karisanno11 · 2023-11-15T20:52:20Z

cves/kernel/CVE-2016-9754.yml

 upvotes_instructions: |
  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: 3


Overall great! Your explanations are clear and easy to understand. The minor errors are consistency in capitalization and punctuation across the document.
I would give this a 5.

karisanno11 · 2023-11-15T21:04:49Z

cves/kernel/CVE-2016-9754.yml

@@ -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:


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

Update Curation Level to 2

e268ed5

andymeneely added the cohort T label Nov 11, 2023

evstod added 9 commits November 11, 2023 14:16

CVE-2022-0516 yml filled

ed022af

Remove unintned upvotes

2c1bfa7

CVE-2016-9754 yml filled

53cbbf9

Fix tabbing in CVE-2022-0516 interesting_commits

f44635f

fix booleans to succeed checks

b4b960e

fix unit_tested, sandbox, i18n, and ipc notes

28a4b0c

Fix vccs quotes

6316e0f

fix specification answer

773e65c

fix autodiscoverable answer

34ee0ca

mineo333 reviewed Nov 12, 2023

View reviewed changes

evstod and others added 8 commits November 13, 2023 14:14

CVE-2016-9754 specifications typo fix

29c9f41

Co-authored-by: Sharad Khanna <[email protected]>

Redefine "buffer overflow" as "integer overflow

1175637

While the vuln regards an int that controls a buffer size, it is not a buffer overflow

Fix missing char in VCC commit id CVE-2022-0516

5087cc0

fix broken fix commit id

e0c2ff1

CVE-2016-9754 change ipc answer to false.

afd6a55

Revert "CVE-2016-9754 change ipc answer to false."

ed241d3

This reverts commit afd6a55.

Change upvotes to 2

6583634

Add upvotes

997a01f

JoeWesn reviewed Nov 15, 2023

View reviewed changes

Patrick-Sorensen reviewed Nov 15, 2023

View reviewed changes

Update upvotes

82cd6a9

karisanno11 reviewed Nov 15, 2023

View reviewed changes

upvotes & type fixes

86d3715

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CVE-2016-9754 and CVE-2022-0516 #225

CVE-2016-9754 and CVE-2022-0516 #225

evstod commented Nov 10, 2023

andymeneely commented Nov 11, 2023

mineo333 left a comment

mineo333 Nov 12, 2023

evstod Nov 13, 2023 •

edited

Loading

mineo333 Nov 12, 2023

evstod Nov 13, 2023

mineo333 Nov 12, 2023

evstod Nov 13, 2023

mineo333 Nov 12, 2023

evstod Nov 13, 2023

mineo333 Nov 12, 2023

mineo333 Nov 12, 2023

JoeWesn Nov 15, 2023

evstod Nov 15, 2023

JoeWesn Nov 15, 2023

JoeWesn Nov 15, 2023

JoeWesn Nov 15, 2023

JoeWesn Nov 15, 2023

JoeWesn Nov 15, 2023

evstod Nov 15, 2023

Patrick-Sorensen left a comment

Patrick-Sorensen Nov 15, 2023

Patrick-Sorensen Nov 15, 2023

Patrick-Sorensen Nov 15, 2023

karisanno11 Nov 15, 2023

karisanno11 Nov 15, 2023

karisanno11 Nov 15, 2023

karisanno11 Nov 15, 2023 •

edited

Loading

karisanno11 Nov 15, 2023

karisanno11 Nov 15, 2023

karisanno11 Nov 15, 2023

CVE-2016-9754 and CVE-2022-0516 #225

Are you sure you want to change the base?

CVE-2016-9754 and CVE-2022-0516 #225

Conversation

evstod commented Nov 10, 2023

andymeneely commented Nov 11, 2023

mineo333 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evstod Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Patrick-Sorensen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karisanno11 Nov 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evstod Nov 13, 2023 •

edited

Loading

karisanno11 Nov 15, 2023 •

edited

Loading