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
111 changes: 53 additions & 58 deletions cves/kernel/CVE-2012-6657.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ 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
Expand Down Expand Up @@ -55,7 +55,8 @@ description_instructions: |

Your target audience is people just like you before you took any course in
security
description:
description: The function sock_setsockopt does not check the type of socket before running tcp_set_keepalive
on it, users could use a RAW socket to crash the system.

Choose a reason for hiding this comment

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

This is a good start, but consider going into more detail here. Especially could explain a bit better:

  • What is sock_setsockopt and what was it used for?
  • What is tcp_set_keepalive and why is it a problem if the socket type is not checked?
  • What is a RAW socket and how would it result in a system crash?

From the question description, this section should be explained as if you don't have a background in security. From just reading this response it's not quite clear what the vulnerability was. Adding details in layman's terms describing what this issue actually was would be more helpful for future readers.

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 Down Expand Up @@ -84,14 +85,8 @@ fixes_instructions: |

Place any notes you would like to make in the notes field.
fixes:
- commit:
note:
- commit:
note:
- commit: 3e10986d1d698140747fcfc2761ec9cb64c1d582
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'
note: Manually Confirmed
vcc_instructions: |
The vulnerability-contributing commits.

Expand All @@ -106,15 +101,15 @@ vcc_instructions: |
Place any notes you would like to make in the notes field.
vccs:
- commit: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
note: Discovered automatically by archeogit.
note: Manually verified
upvotes_instructions: |

Choose a reason for hiding this comment

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

For this vulnerability, I'd give it a 2.

Copy link

Choose a reason for hiding this comment

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

2 upvotes

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: 7
unit_tested:
question: |
Were automated unit tests involved in this vulnerability?
Expand All @@ -129,10 +124,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 automated unit tests were involved.
fix: false
fix_answer: Only the change to the function was part of the commit.
discovered:
question: |
How was this vulnerability discovered?
Expand All @@ -147,10 +142,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: Discovered by a developer, no used of tools were mentioned.

Choose a reason for hiding this comment

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

Small rewording change could be made here for clarification

Suggested change
answer: Discovered by a developer, no used of tools were mentioned.
answer: Discovered by a developer and no use of automated tools was mentioned.

automated: false
contest: false
developer: false

Choose a reason for hiding this comment

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

The fix commit was signed off by a google employee so the developer flag should be true

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

The answer field should be boolean. In answer_note, please explain
why you come to that conclusion.
note:
answer:
note: The vulnerability relies on a specific type of socket to be used,
a fully automated tool could not find the issue.
answer: false
specification:
instructions: |
Is there mention of a violation of a specification? For example, the POSIX
Expand All @@ -184,8 +180,8 @@ specification:

The answer field should be boolean. In answer_note, please explain
why you come to that conclusion.
note:
answer:
note: No mention of a violation of a specification.
answer: false
subsystem:
question: |
What subsystems was the mistake in? These are WITHIN linux kernel
Expand Down Expand Up @@ -219,7 +215,7 @@ subsystem:
e.g.
name: ["subsystemA", "subsystemB"] # ok
name: subsystemA # also ok
name:
name: net
note:
interesting_commits:
question: |
Expand All @@ -237,8 +233,6 @@ interesting_commits:
commits:
- commit:
note:
- commit:
note:
i18n:
question: |
Was the feature impacted by this vulnerability about internationalization
Expand All @@ -251,8 +245,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: Issue was purely about socket types.
sandbox:
question: |
Did this vulnerability violate a sandboxing feature that the system
Expand All @@ -266,8 +260,9 @@ 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: true
note: The funtion in question (sock_setsockopt) is used to control socket behavior.
It was not properly checking if the socket was RAW
ipc:
question: |
Did the feature that this vulnerability affected use inter-process
Expand All @@ -278,8 +273,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: false
note: Not part of IPC
Copy link

Choose a reason for hiding this comment

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

How so? It is my understanding that sockets are inherently inter-process.

Choose a reason for hiding this comment

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

I believe this should instead be true, as the vulnerability was caused by an improper passing of sockets which is an IPC mechanism

discussion:
question: |
Was there any discussion surrounding this?
Expand All @@ -305,9 +300,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: First mention was that it was an issue and required a quick fix.
vouch:
question: |
Was there any part of the fix that involved one person vouching for
Expand All @@ -320,8 +315,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: false
note: n/a
Copy link

Choose a reason for hiding this comment

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

Check the commit message. I see two acknowledgements of people that signed off.

Choose a reason for hiding this comment

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

I believe this would be true, as the fix commit has three developers who reviewed and signed off on it
torvalds/linux@3e10986

stacktrace:
question: |
Are there any stacktraces in the bug reports?
Expand All @@ -335,9 +330,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: No stacktrace given in report
forgotten_check:
question: |
Does the fix for the vulnerability involve adding a forgotten check?
Expand All @@ -356,8 +351,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: true
note: Yes, system forgets to check if Socket is RAW.
order_of_operations:
question: |
Does the fix for the vulnerability involve correcting an order of
Expand All @@ -369,8 +364,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: false
note: Fix adds one new check.
lessons:
question: |
Are there any common lessons we have learned from class that apply to this
Expand All @@ -387,37 +382,37 @@ 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:
least_privilege:
applies:
applies: false
note:
frameworks_are_optional:
applies:
applies: false
note:
native_wrappers:
applies:
applies: false
note:
distrust_input:
applies:
applies: false
Copy link

Choose a reason for hiding this comment

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

The socket type being sent by the user/process counts as input. This input is not being checked fully. I would propose this to count under distrust_input.

note:
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:
applies: false
note:
mistakes:
question: |
Expand Down Expand Up @@ -448,7 +443,7 @@ mistakes:

Write a thoughtful entry here that people in the software engineering
industry would find interesting.
answer:
answer: lapse - Forgot to check if socket was RAW
CWE_instructions: |
Comment on lines +446 to 447

Choose a reason for hiding this comment

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

Try and go more in depth about the specifics of this issue.

Please go to http://cwe.mitre.org and find the most specific, appropriate CWE
Copy link

Choose a reason for hiding this comment

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

CWE 264 is a category. It may be worth looking for a specific CWE in that category.

entry that describes your vulnerability. We recommend going to
Expand All @@ -473,5 +468,5 @@ 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:
CVSS:
nickname: Socket type non check.
CVSS:

Choose a reason for hiding this comment

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

Looks like this vulnerability has not yet been provided a 3.1 CVSS score, but instead you could use the 2.0 CVSS from "https://nvd.nist.gov/vuln/detail/CVE-2012-6657"

Suggested change
CVSS:
CVSS: CVSS:2.0AV:L/AC:L/Au:N/C:N/I:N/A:C

Loading