log2timeline / plaso

Super timeline all the things
https://plaso.readthedocs.io
Apache License 2.0
1.66k stars 327 forks source link

Allowed launchd plists missing optional fields #4803

Closed Spferical closed 3 months ago

Spferical commented 4 months ago

One line description of pull request

This fixes parsing of launchd plists that are missing optional fields.

Description:

I noticed many launchd plists failing to emit any events. According to apple docs, Label and ProgramArguments are required. In practice, it looks like ProgramArguments is also optional, with many LaunchDaemon plists on my macos 13.6 system having only Program set.

Notes:

All contributions to Plaso undergo code review. This makes sure that the code has appropriate test coverage and conforms to the Plaso style guide.

One of the maintainers will examine your code, and may request changes. Check off the items below in order, and then a maintainer will review your code.

Checklist:

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 76.47059% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 85.16%. Comparing base (25b533b) to head (40432ed). Report is 1 commits behind head on main.

Files Patch % Lines
plaso/parsers/plist.py 60.00% 2 Missing :warning:
plaso/parsers/plist_plugins/launchd.py 77.77% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #4803 +/- ## ========================================== - Coverage 85.16% 85.16% -0.01% ========================================== Files 427 427 Lines 38614 38623 +9 ========================================== + Hits 32886 32893 +7 - Misses 5728 5730 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

joachimmetz commented 4 months ago

Spferical requested a review from joachimmetz 7 hours ago

I'll take another look when time permits

joachimmetz commented 3 months ago

https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/BPSystemStartup/Chapters/CreatingLaunchdJobs.html section "Required and recommended property list keys" indicates ProgramArguments is also a required key

And I see you mention this in your original post

In practice, it looks like ProgramArguments is also optional, with many LaunchDaemon plists on my macos 13.6 system having only Program set.

joachimmetz commented 3 months ago

@Spferical PTAL if the additional changes match your requirements

Spferical commented 3 months ago

@Spferical PTAL if the additional changes match your requirements

Looks great to me, thank you!