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

Fix UTMP misinterpretation of IPv6 addresses #292

Merged
merged 22 commits into from
Aug 22, 2023

Conversation

Zawadidone
Copy link
Contributor

@Zawadidone Zawadidone commented Jun 26, 2023

Closes #291

@Zawadidone
Copy link
Contributor Author

What is the best place for this code? So that the code can be re-used by the BTMP parser.

@Zawadidone Zawadidone changed the title Fix UTMP IPv6 Fix UTMP misinterpretation of IPv6 addresses Jun 26, 2023
@Schamper
Copy link
Member

What is the best place for this code? So that the code can be re-used by the BTMP parser.

Perhaps here, and yield some named tuples with this pre-parsed field instead of the direct cstruct entry? https://github.com/fox-it/dissect.target/blob/main/dissect/target/plugins/os/unix/log/utmp.py

dissect/target/plugins/os/unix/log/utmp.py Outdated Show resolved Hide resolved
dissect/target/plugins/os/unix/log/utmp.py Outdated Show resolved Hide resolved
dissect/target/plugins/os/unix/log/utmp.py Outdated Show resolved Hide resolved
@Zawadidone
Copy link
Contributor Author

@cecinestpasunepipe should the files wtmp.py and btmp.py be combined in utmp.py with the namespace utmp with the functiosn wtmp and btmp? Because the code is identical and only the path and record name is different.

@Schamper
Copy link
Member

@cecinestpasunepipe should the files wtmp.py and btmp.py be combined in utmp.py with the namespace utmp with the functiosn wtmp and btmp? Because the code is identical and only the path and record name is different.

You could also just put a UtmpPlugin in utmp.py with a function btmp and wtmp, skipping the namespace.

@Zawadidone
Copy link
Contributor Author

Okay I have combined the functions

pyrco
pyrco previously approved these changes Jul 28, 2023
@Zawadidone
Copy link
Contributor Author

@cecinestpasunepipe can you take look at this PR?

@cecinestpasunepipe
Copy link
Contributor

Okay I have combined the functions

Yes, I will review this PR right now.

dissect/target/plugins/os/unix/log/utmp.py Outdated Show resolved Hide resolved
dissect/target/plugins/os/unix/log/utmp.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #292 (a9261f8) into main (f895370) will increase coverage by 0.02%.
The diff coverage is 96.66%.

@@            Coverage Diff             @@
##             main     #292      +/-   ##
==========================================
+ Coverage   71.87%   71.89%   +0.02%     
==========================================
  Files         238      236       -2     
  Lines       18717    18725       +8     
==========================================
+ Hits        13452    13463      +11     
+ Misses       5265     5262       -3     
Flag Coverage Δ
unittests 71.89% <96.66%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
dissect/target/plugins/os/unix/log/utmp.py 94.59% <96.66%> (+4.59%) ⬆️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@cecinestpasunepipe cecinestpasunepipe left a comment

Choose a reason for hiding this comment

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

LGTM

@Zawadidone
Copy link
Contributor Author

Zawadidone commented Aug 22, 2023

@pyrco @cecinestpasunepipe thank you for reviewing the PR and coming with suggestions to fix the problem.

@pyrco pyrco merged commit b85df6b into fox-it:main Aug 22, 2023
10 checks passed
@Zawadidone Zawadidone deleted the fix/utmp_ipv6 branch August 22, 2023 08:47
Zawadidone added a commit to Zawadidone/dissect.target that referenced this pull request Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misinterpretation of IPv6 addresses (UTMP)
4 participants