Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(968)

Issue 327070043: [plaso] Add amcache parser #740

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 4 days ago by rbdebeer
Modified:
3 days, 23 hours ago
Reviewers:
jberggren, onager
CC:
Joachim Metz, romaing, kiddi, log2timeline-dev_googlegroups.com, aaronp
Visibility:
Public.

Description

[plaso] Add amcache parser #740

Patch Set 1 #

Total comments: 13
Unified diffs Side-by-side diffs Delta from patch set Stats (+170 lines, -0 lines) Patch
M plaso/formatters/__init__.py View 1 chunk +1 line, -0 lines 0 comments Download
A plaso/formatters/amcache.py View 1 chunk +23 lines, -0 lines 0 comments Download
M plaso/parsers/__init__.py View 1 chunk +1 line, -0 lines 0 comments Download
A plaso/parsers/amcache.py View 1 chunk +103 lines, -0 lines 9 comments Download
M plaso/parsers/presets.py View 1 chunk +2 lines, -1 line 0 comments Download
A test_data/Amcache.hve View 0 chunks +-1 lines, --1 lines 0 comments Download
A tests/parsers/amcache.py View 1 chunk +41 lines, -0 lines 4 comments Download

Messages

Total messages: 2
rbdebeer
1 week, 4 days ago (2017-08-09 09:56:41 UTC) #1
onager
1 week, 4 days ago (2017-08-09 10:38:17 UTC) #2
https://codereview.appspot.com/327070043/diff/1/plaso/parsers/amcache.py
File plaso/parsers/amcache.py (right):

https://codereview.appspot.com/327070043/diff/1/plaso/parsers/amcache.py#newc...
plaso/parsers/amcache.py:3: 
add "from __future__ import unicode_literals" and drop all 'u' prefixes on
strings

https://codereview.appspot.com/327070043/diff/1/plaso/parsers/amcache.py#newc...
plaso/parsers/amcache.py:16: """Amcache event data.
+blank line

https://codereview.appspot.com/327070043/diff/1/plaso/parsers/amcache.py#newc...
plaso/parsers/amcache.py:18: amcachedatetime (str): last modified time of
amcache file referenced
This doesn't need to be in the eventdata, it's in the event instead.

https://codereview.appspot.com/327070043/diff/1/plaso/parsers/amcache.py#newc...
plaso/parsers/amcache.py:20: sha1 (str): sha1 of file
Why not extract all the data from the key? Product name, language code etc.? It
looks pretty trivial to do so.

It also like it would be useful to include the volume guid, ntfs file id and
sequence number as well.

https://codereview.appspot.com/327070043/diff/1/plaso/parsers/amcache.py#newc...
plaso/parsers/amcache.py:42: _SOURCE_APPEND = u': Amcache Entries'
Remove this - source_append is going away, and this value doesn't do anything
anyway, you'd need to add is an attribute to the event data.

https://codereview.appspot.com/327070043/diff/1/plaso/parsers/amcache.py#newc...
plaso/parsers/amcache.py:47: _AMCACHE_ROOT_KEY = "Root\\File"
What about Root\\Programs? It also looks like it might have some useful data.

https://codereview.appspot.com/327070043/diff/1/plaso/parsers/amcache.py#newc...
plaso/parsers/amcache.py:50: #TODO Add GetFormatSpecification when issues are
fixed with adding
Please open a github issue for this, and reference it in the todo

https://codereview.appspot.com/327070043/diff/1/plaso/parsers/amcache.py#newc...
plaso/parsers/amcache.py:66: except IOError as exception: #pylint:
disable=unused-variable
you can just "except IOError" here, and avoid the pylint directive.

Please also add a comment about why this error is ignored, referencing the TODO
above.

https://codereview.appspot.com/327070043/diff/1/plaso/parsers/amcache.py#newc...
plaso/parsers/amcache.py:83: for amcache_key in range(amcache_keys):
This seems a little wierd - why not "for amcache_key in volume_key.sub_keys"?

https://codereview.appspot.com/327070043/diff/1/tests/parsers/amcache.py
File tests/parsers/amcache.py (right):

https://codereview.appspot.com/327070043/diff/1/tests/parsers/amcache.py#newc...
tests/parsers/amcache.py:4: 
Add "from __future__ import unicode_iterals", and then drop all the 'u' prefixes
from strings.

https://codereview.appspot.com/327070043/diff/1/tests/parsers/amcache.py#newc...
tests/parsers/amcache.py:17:
@shared_test_lib.skipUnlessHasTestFile([u'Amcache.hve'])
Please also add a test for parsing a non-matching file - ideally a non-amcache
registry file.

https://codereview.appspot.com/327070043/diff/1/tests/parsers/amcache.py#newc...
tests/parsers/amcache.py:33: expected_full_path =
(u'c:\\users\\user\\appdata\\local\\microsoft\\'
Line break after (

https://codereview.appspot.com/327070043/diff/1/tests/parsers/amcache.py#newc...
tests/parsers/amcache.py:37: expected_sha1 =
u'0000818b581a471c1c6833839d35a9d6f3544f6a9c92'
This doesn't look like a sha1 - 43 characters long, due to leading zeros. I
recommend stripping the extra zeros off here, or renaming the field.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted