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.

Sign up for GitHub

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

Jump to bottom

Do not yield DRIVER_OBJECT instances found in scanning that are not a… #1488

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Do not yield DRIVER_OBJECT instances found in scanning that are not a… #1488

wants to merge 3 commits into from

Conversation

atcuno
Copy link
Contributor

@atcuno atcuno commented Dec 28, 2024

…ctual instances #1481

@atcuno atcuno requested a review from ikelos December 28, 2024 22:13
@ikelos ikelos linked an issue Dec 30, 2024 that may be closed by this pull request
MajorFunction access crashes driverirp #1481
Open
ikelos
ikelos reviewed Dec 30, 2024
View reviewed changes
Copy link
Member

@ikelos ikelos left a comment

Choose a reason for hiding this comment

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

Never seen a variable just accessed without an assignment or anything, but if it works? Just want to check that getting back a structure with only one of those is still worthwhile...

volatility3/framework/plugins/windows/driverscan.py Outdated
# of interest to plugins, so if it is not accessible then this instance
# is not useful or usable during analysis
try:
mem_object.DriverStart
Copy link
Member

Choose a reason for hiding this comment

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

Is just listing a variable enough to trip the output? I'd think that could get optimized out? Also, don't we care if the whole structure is valid, or would having just DriverStart be enough?

@atcuno atcuno mentioned this pull request Jan 24, 2025
Open
14 tasks
@atcuno atcuno force-pushed the issue_1481_driver_objects branch from 49dbb01 to 8df8f77 Compare January 24, 2025 21:12
@atcuno
Copy link
Contributor Author

atcuno commented Jan 24, 2025

@ikelos this is ready but not sure if a version bump is needed due to the error paths in get_names_for_driver changing to meet how we code things now. The function didn't type the return value before so that part didn't change. The only place in the current code base that externally calls it is drivermodule, which is going to change for other fixes right after this is merged anyway, so its an easy fix if needed.

@ikelos
Copy link
Member

ikelos commented Jan 25, 2025

Looks like some tests need updating here? I dunno whether we lost legit values or something else went wrong, but it needs someone to look into it. Technically it should get a bump given the return type is changed. If it's not too hard to put it, it would probably be worthwhile, just in case it does affect someone...

@ikelos ikelos assigned atcuno and unassigned ikelos Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ikelos ikelos ikelos left review comments

Assignees

@atcuno atcuno

Labels
awaiting-author-fixes parity-release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

MajorFunction access crashes driverirp
2 participants
@atcuno @ikelos