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

Add exception handling in mountinfo enumeration path #1515

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Add exception handling in mountinfo enumeration path #1515

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 10 additions & 3 deletions volatility3/framework/plugins/linux/mountinfo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from collections import namedtuple
from typing import Tuple, List, Iterable, Union

from volatility3.framework import renderers, interfaces
from volatility3.framework import renderers, interfaces, exceptions
from volatility3.framework.configuration import requirements
from volatility3.framework.interfaces import plugins
from volatility3.framework.symbols import linux
Expand Down Expand Up @@ -96,9 +96,16 @@ def get_mountinfo(
superblock = mnt.get_mnt_sb()

mnt_id: int = mnt.mnt_id
parent_id: int = mnt.mnt_parent.mnt_id

st_dev = f"{superblock.major}:{superblock.minor}"
# Many samples in mass testing backtraced from
# invalid superblocks being return
# mnt_parent can also smear so we check both
try:
parent_id: int = mnt.mnt_parent.mnt_id
Copy link
Contributor

Choose a reason for hiding this comment

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

mnt_parent is a pointer.. this should be:

if not (mnt.mnt_parent and mnt.mnt_parent.is_readable()):
    return None

Same thing for superblock:

    def get_mnt_sb(self) -> int:
        """Returns a pointer to the super_block"""

Copy link
Member

Choose a reason for hiding this comment

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

I would generally err towards testing before accepting (especially if there's gonna be extra code required either way). I've tried in this project to be a little happier about exception handling, although ideally they'd only occur in exceptional circumstances not expected ones. The good part about exception handling is that the try/catch can cover much larger chunks of code regardless of what exactly the issue was, so I'm trying to encourage try/except blocks that cover as much as could go wrong if something were to go wrong (but no more than that) so usually the body of a loop for instance, or something of that nature.

I'm gonna leave you two to figure out what's best ask permission/seek forgiveness, once you come to a consensus let me know, at the moment either one seems ok to me unless someone presents an argument as to why one is better or actively detrimental...


st_dev = f"{superblock.major}:{superblock.minor}"
except exceptions.InvalidAddressException:
return None

mnt_opts: List[str] = []
mnt_opts.append(mnt.get_flags_access())
Expand Down
Loading