Plist recursekey fix

[oh6hay]

One line description of pull request

Fixes the plist parser hanging for an exponentially long time attempting to recurse a valid PLIST that contains a very large number of unique paths to a small number of leaf nodes, using shared references

Description:

This fixes an issue with the plist parser interface's _RecurseKey method. For list elements, the _RecurseKey recurses into each list element independently, not checking if the references in a list point to a target already recursed into. This allows constructing a bplist representing a chain of lists, so that each list contains multiple references to the next element in the chain. The _RecurseKey naively recurses into each reference independently, not checking if it traverses into a node already traversed. The depth check doesn't help here as 15 is plenty of levels to construct an exponentially large amount of unique paths in the chain. This is fixed by also passing a set of already visited objects on the method call. The change also protects against cyclic references, as a treat.

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:

• No new new dependencies are required or l2tdevtools has been updated.
• Test data has a Plaso compatible license. If the test data was not authored by you (the contributor), make sure to mention its original source in ACKNOWLEDGEMENTS.
• Reviewer assigned.
• Automated checks (GitHub Actions, AppVeyor) pass.

[codecov]

Codecov Report

❌ Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.06%. Comparing base (3ea94cf) to head (a6ae725).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
plaso/parsers/plist_plugins/interface.py	63.63%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5108      +/-   ##
==========================================
- Coverage   85.07%   85.06%   -0.01%     
==========================================
  Files         454      454              
  Lines       40221    40231      +10     
==========================================
+ Hits        34217    34223       +6     
- Misses       6004     6008       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[oh6hay]

anything i can do to help this be reviewed?

[joachimmetz]

Will take a look as soon as time permits

[joachimmetz]

Where is the test file datetime_multipath.bplist used?

Your solution is checking runtime object ids for previous visited and cyclic references, under which conditions would these surface?

I'm also not in favor of silently skipping previously visited objects. Couldn't these have a different meaning in a different plist context? Might be better to detect this format edge case, issue a parser warning and abort processing the file.