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-2013-7348 and CVE-2022-3105 #204

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open
Changes from 1 commit
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
Prev Previous commit
  • Loading branch information
as9215 authored Nov 7, 2023
commit dd84faf70cc84f59a99ca3305928df191e4da9ad
99 changes: 47 additions & 52 deletions CVE-2022-3105.yml

Choose a reason for hiding this comment

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

Overall, I agree with most of what the other commenters have added here. The only place I'd disagree is that I like how you break down some concepts, as these notes are meant for someone with less programming experience than us. I'd make sure you add the description of the vulnerability as I don't actually know what it is. I know it deals with communication and drivers but that's about it. I'd recommend re-forking as the format of this file is different from the main repository and you're missing some key points.

Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ CVE: CVE-2022-3105
CWE:
- 476
ipc:
note:
answer:
note: This is primarily used for communication between user-space applications and kernel-space InfiniBand (IB) and Remote Direct Memory Access (RDMA) drivers. It doesn't directly deal with inter-process communication (IPC) mechanisms such as OS signals, pipes, stdin/stdout, message passing, clipboard, or file-based IPC.
Copy link

Choose a reason for hiding this comment

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

I think you can focus on what the vulnerability does and you don't need to reiterate IPC because we should all know what it is at this point

answer: False
question: |
Did the feature that this vulnerability affected use inter-process
communication? IPC includes OS signals, pipes, stdin/stdout, message
Expand All @@ -14,14 +14,14 @@ ipc:
Write a note about how you came to the conclusions you did, regardless of
what your answer was.
CVSS: CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:N/I:N/A:H
bugs: []
bugs: [2153067]
i18n:
note:
answer:
note: It is unlikely that the vulnerabilities or issues related to this code have a direct impact on internationalization (i18n) features. Internationalization features in software are generally concerned with providing support for different languages, character encodings, locales, and user interface translations, among other aspects.
Copy link

Choose a reason for hiding this comment

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

Once again I don't think you need to define i18n in your answer.

answer: False
question: |
Was the feature impacted by this vulnerability about internationalization
(i18n)?
s

An internationalization feature is one that enables people from all
over the world to use the system. This includes translations, locales,
typography, unicode, or various other features.
Expand All @@ -37,14 +37,11 @@ fixes:
commit:
- note:
commit:
- 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

Choose a reason for hiding this comment

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

I would recommend leaving a short note describing what this commit does, just a few words for the reader to know how it fixes the vulnerability.

commit: 7694a7de22c53a312ea98960fcafc6ec62046531
vouch:
note:
answer:
note: I checked the kernel.org hyperlink in the references section of nvd
answer: True
question: >
Was there any part of the fix that involved one person vouching for

Expand All @@ -66,7 +63,7 @@ bounty:
lessons:
yagni:
note:
applies:
applies: [distrust_input, complex_input]

Choose a reason for hiding this comment

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

I think yagni is a lesson itself, you'd probably just put "false" here if it doesn't apply

question: |
Are there any common lessons we have learned from class that apply to this
vulnerability? In other words, could this vulnerability serve as an example
Expand All @@ -83,38 +80,38 @@ lessons:
free to give it a small name and add one in the same format as these.
serial_killer:
note:
applies:
applies: False
complex_inputs:
note:
applies:
note: error handling can have errors smoothly handled if a null pointer is dereferenced
applies: True
distrust_input:
note:
applies:
note: input validation can be used to avoid a null pointer dereference
applies: True
least_privilege:
note:
applies:
applies: False
native_wrappers:
note:
applies:
applies: False
defense_in_depth:
note:
applies:
applies: False
secure_by_default:
note:
applies:
applies: False
environment_variables:
note:
applies:
applies: False
security_by_obscurity:
note:
applies:
applies: False
frameworks_are_optional:
note:
applies:
applies: False
reviews: []
sandbox:
note:
answer:
note: This vulnerability relates more to memory allocation and error handling as opposed to sandboxing violations or access control issues.
answer: False
question: |
Did this vulnerability violate a sandboxing feature that the system
provides?
Expand All @@ -128,11 +125,10 @@ sandbox:
Write a note about how you came to the conclusions you did, regardless of
what your answer was.
upvotes: 0
CWE_note: |
CWE as registered in the NVD. If you are curating, check that this
is correct and replace this comment with "Manually confirmed".
CWE_note: Manually confirmed
mistakes:
answer:
answer: The vulnerability stems from coding mistakes and design flaws. The critical coding mistake lies in the failure to check the result of a memory allocation operation, specifically, the use of kmalloc_array. This error handling oversight can be attributed to a lack of attention to the return value of the allocation function. Design mistakes may also be relevant if there are patterns of a lack of error handling and memory management throughout the code. Miscommunications could have contributed improper error handling practices in the code but there is no evidence that miscommunication is the direct cause.

question: |
In your opinion, after all of this research, what mistakes were made that
led to this vulnerability? Coding mistakes? Design mistakes?
Expand Down Expand Up @@ -161,10 +157,10 @@ mistakes:

Write a thoughtful entry here that people in the software engineering
industry would find interesting.
nickname:
nickname: NULL Pointer Dereference
subsystem:
name:
note:
name: RDMA
note: this is known because of the headers directly referencing rdma, the function names, and the subsystem references.
question: >
What subsystems was the mistake in? These are WITHIN linux kernel

Expand Down Expand Up @@ -203,8 +199,8 @@ subsystem:
name: ["subsystemA", "subsystemB"] # ok
name: subsystemA # also ok
discovered:
answer:
contest:
answer: 2022-12-13
contest: False
question: |
How was this vulnerability discovered?

Expand All @@ -218,10 +214,10 @@ discovered:

If there is no evidence as to how this vulnerability was found, then please
explain where you looked.
automated:
developer:
automated: False
developer: True
Copy link

Choose a reason for hiding this comment

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

I believe you need to put the developers name instead of true here

discussion:
note:
note: I was not able to find any disagreements
Copy link

Choose a reason for hiding this comment

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

You marked discussion and security as true though?

question: |
Was there any discussion surrounding this?

Expand All @@ -246,8 +242,8 @@ discussion:

Put any links to disagreements you found in the notes section, or any other
comment you want to make.
any_discussion:
discussed_as_security:
any_discussion: True
discussed_as_security: True
stacktrace:
Copy link

Choose a reason for hiding this comment

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

I think you just forgot this one. Even if you didn't find a stacktrace you should still put false down

note:
question: |
Expand All @@ -267,7 +263,7 @@ stacktrace:
description:

Choose a reason for hiding this comment

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

Make sure to add a description of the vulnerability here, like I mentioned in your other CVE

unit_tested:
fix:
Copy link

Choose a reason for hiding this comment

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

Since you found a unit test was it good was it bad? Do you know who made it? Was it made before or after the cause of the vulnerability but before the fix? You don't have to answer all my questions exactly but I think they'd be good to think about.

code:
code: True
question: |
Were automated unit tests involved in this vulnerability?
Was the original code unit tested, or not unit tested? Did the fix involve
Expand All @@ -283,10 +279,10 @@ unit_tested:
adding or improving an automated test to ensure this doesn't happen again.
fix_answer:
code_answer:
reported_date:
reported_date: 2022-12-13
specification:
note:
answer:
note: POSIX may be violated by failing to handle memory allocation errors.
answer: True
instructions: |
Is there mention of a violation of a specification? For example, the POSIX
spec, an RFC spec, a network protocol spec, or some other requirements
Expand All @@ -305,8 +301,8 @@ announced_date: 2022-12-14
curation_level: 2
published_date: 2022-12-14
forgotten_check:
note:
answer:
note: This is shown in the kernel.org fix demonstrated by jiasheng jiang. According to the log, the author inserted code to check the data variable.
answer: True
question: |
Does the fix for the vulnerability involve adding a forgotten check?

Expand All @@ -325,8 +321,8 @@ forgotten_check:
Write a note about how you came to the conclusions you did, regardless of
what your answer was.
autodiscoverable:
note:
answer:
note: An empty dataset would trigger this vulnerability
Copy link

Choose a reason for hiding this comment

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

Would a automated tool be able to interpret the result it gets from this?

answer: True
instructions: |
Is it plausible that a fully automated tool could have discovered
this? These are tools that require little knowledge of the domain,
Expand Down Expand Up @@ -361,8 +357,8 @@ interesting_commits:
* Other commits that fixed a similar issue as this vulnerability

Choose a reason for hiding this comment

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

I'd recommend adding a commit or two in the interesting commits section or at least mentioning why there weren't any (they were trivial, there weren't any at all, etc.)

* Anything else you find interesting.
order_of_operations:
note:
answer:
note: The fix requires adding a check for memory allocation prior to dereferencing
answer: False
question: |
Does the fix for the vulnerability involve correcting an order of
operations?
Expand All @@ -373,4 +369,3 @@ 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.