CVE-2017-5548 and CVE-2016-5728 #198

am7504 · 2023-11-06T02:06:39Z

Completed investigations for both CVEs assigned to me.

nolan-white · 2023-11-10T20:42:06Z

cves/kernel/CVE-2016-5728.yml

-  answer:
-  note:
+  answer: True
+  note: Two people had signed off on the commit that fixed the issue.


Who were the developers that signed off on the fix?

nolan-white · 2023-11-10T20:44:07Z

cves/kernel/CVE-2016-5728.yml

@@ -184,8 +193,10 @@ specification:

    The answer field should be boolean. In answer_note, please explain
    why you come to that conclusion.
-  note:
-  answer:
+  note: |


I would suggest changing the writing perspective here to third-person.

nolan-white · 2023-11-10T20:52:19Z

cves/kernel/CVE-2016-5728.yml

@@ -55,7 +55,11 @@ description_instructions: |

  Your target audience is people just like you before you took any course in
  security
-description:
+description: A race condition in one of the functions in a certain driver that allows 


I think readers would appreciate context about the driver the vulnerability existed in. It would make it feel more real and interesting.

I agree with this assessment; however, it's important to keep in mind that the description is not supposed to contain information that is too far beyond our target audience and should not include terms that people outside the project would fail to understand.

nolan-white · 2023-11-10T20:59:07Z

cves/kernel/CVE-2016-5728.yml

-description:
+description: A race condition in one of the functions in a certain driver that allows 
+  users to obtain senstive information from memory or cause memory corruption or a system 
+  crash. If the user modifies the header of a file, the function might incorrectly fetch 


Were any examples given of how a user could modify the header file could be modified to cause this vulnerability? The way it is described here sounds to me like a developer would have to update the header source code file, but that wouldn't affect code until the program is recompiled so it leaves me wondering how this would occur.

nolan-white · 2023-11-10T21:00:37Z

cves/kernel/CVE-2016-5728.yml

-  contest:
-  developer:
-autodiscoverable:
+  answer: |


Who is "them"? The developers or an outside source? If it's the developers the developer field should probably be updated to true.

nolan-white · 2023-11-10T21:04:04Z

cves/kernel/CVE-2016-5728.yml

+  answer: True
+  note: |
+    This fix involves changing a variable if the check was passed. Once this variable is 
+    changed to a certain value, it can then be written over correclty, or get used with the 


Typo on line 399 in "correclty".

nolan-white · 2023-11-10T21:06:54Z

cves/kernel/CVE-2016-5728.yml

@@ -448,7 +475,10 @@ mistakes:

    Write a thoughtful entry here that people in the software engineering
    industry would find interesting.
-  answer:
+  answer: | 


The first sentence feels like a bullet point. I would suggest changing this to flow like a paragraph all the way through.

nolan-white · 2023-11-11T22:22:43Z

cves/kernel/CVE-2016-5728.yml

NamesAreDifficult · 2023-11-13T01:24:30Z

2 Upvotes each.

NamesAreDifficult · 2023-11-13T01:06:06Z

cves/kernel/CVE-2016-5728.yml

@@ -55,7 +55,11 @@ description_instructions: |

  Your target audience is people just like you before you took any course in
  security
-description:
+description: A race condition in one of the functions in a certain driver that allows 


I agree with this assessment; however, it's important to keep in mind that the description is not supposed to contain information that is too far beyond our target audience and should not include terms that people outside the project would fail to understand.

NamesAreDifficult · 2023-11-13T01:09:02Z

cves/kernel/CVE-2016-5728.yml

-  developer:
-autodiscoverable:
+  answer: |
+      '2016-04-18' An email was sent that detailed them finding the bug while examining


Where was this email hosted?

NamesAreDifficult · 2023-11-13T01:10:46Z

cves/kernel/CVE-2016-5728.yml

-  note:
+  answer: False
+  note: | 
+    This function does not comminicate with any other process or system, it's an 


Communicate is misspelled here.

NamesAreDifficult · 2023-11-13T01:11:41Z

cves/kernel/CVE-2016-5728.yml

+  discussed_as_security: True
+  any_discussion: False
+  note: | 
+    This was discussed only one time, but no back and forth discussion. An email was 


Would this qualify more as a report rather than a discussion?

NamesAreDifficult · 2023-11-13T01:12:10Z

cves/kernel/CVE-2016-5728.yml

+  stacktrace_with_fix: False
+  note: | 
+    No stacktraces were found, however there might have been one since the original 
+    person that found the fix knew specific line numbers that pointed out the issues in the 


Good observation here. They may have also debugged the kernel themselves before reporting.

NamesAreDifficult · 2023-11-13T01:17:44Z

cves/kernel/CVE-2017-5548.yml

-  answer:
-  note:
+  answer: True
+  note: Two people signed off on this commit as well as a cc to [email protected].


Who were the people that signed off on this? Was one of them the original developer for the commit?

NamesAreDifficult · 2023-11-13T01:18:35Z

cves/kernel/CVE-2017-5548.yml

+  any_stacktraces: False
+  stacktrace_with_fix: False
+  note: |
+    I tried checking all commits and CVE descriptions as well as dev notes. I did not


Consider changing this to the third person. Consider expanding dev developer for formal documentation.

NamesAreDifficult · 2023-11-13T01:19:37Z

cves/kernel/CVE-2017-5548.yml

-  note:
+  answer: False
+  note: | 
+    Doesn't seem like an order of operations, more of a problem with memory allocation 


Many notes are written in a passive voice. Consider removing phrases such as "Doesn't seem" to instill a more confident tone.

NamesAreDifficult · 2023-11-13T01:21:40Z

cves/kernel/CVE-2017-5548.yml

-    note:
+    applies: True
+    note: | 
+      There seems to be added environmental variable that have been added in order to 


This needs to be changed to either "an added environmental variable" or "added environmental variables" in order to make sense. Not sure if this is trying to refer to one or multiple. Consider changing the wording to remove the second added to something along the lines of "An environmental variable was added in order to fix the memory allocation problems."

NamesAreDifficult · 2023-11-13T01:22:53Z

cves/kernel/CVE-2017-5548.yml

@@ -450,7 +475,10 @@ mistakes:

    Write a thoughtful entry here that people in the software engineering
    industry would find interesting.
-  answer:
+  answer: | 
+    Failing to properly manage memory in the buffer. This can allow people to exploit 


Consider adding a designation wether this was caused primarily by a slip, lapse or planning error.

am7504 added 4 commits November 5, 2023 19:43

completed CVE-2017-5548 Investigation

35ba450

complete CVE-2016-5728 Investigation

5e87bf8

fixed yaml syntax

9cd782f

fixed yaml syntax and answers

8c5b572

andymeneely added the cohort Q label Nov 7, 2023

nolan-white reviewed Nov 10, 2023

View reviewed changes

nolan-white reviewed Nov 11, 2023

View reviewed changes

cves/kernel/CVE-2016-5728.yml

Copy link

nolan-white Nov 11, 2023

Choose a reason for hiding this comment

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

1 upvote

NamesAreDifficult reviewed Nov 13, 2023

View reviewed changes

revised cve records based on feedback

d5753fe

CVE-2017-5548 and CVE-2016-5728 #198

Are you sure you want to change the base?

CVE-2017-5548 and CVE-2016-5728 #198

Uh oh!

Conversation

am7504 commented Nov 6, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nolan-white Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nolan-white Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nolan-white Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NamesAreDifficult commented Nov 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nolan-white Nov 10, 2023 •

edited

Loading

nolan-white Nov 10, 2023 •

edited

Loading

nolan-white Nov 10, 2023 •

edited

Loading