lmco / laikaboss

Laika BOSS: Object Scanning System
Apache License 2.0
739 stars 156 forks source link

adding manifest file parsing module #31

Closed jshlbrd closed 8 years ago

jshlbrd commented 8 years ago

This PR adds a JAR manifest file parsing module. The module relies on python-javatools. Thanks!

jshlbrd commented 8 years ago

Sample output can be seen here: https://github.com/jshlbrd/laikaboss-modules/tree/master/meta_manifest

kglm commented 8 years ago

Nice and simple, beautiful module. Got one recommendation and two suggestions for you.

Recommend: You add the name JAVA to the module name. This provides more context at the global scope for the type of manifest being meta'd. Since you are using javatool's Manifest parser, I assume this is specific to JAR manifests and isn't necessarily reliable for other INF manifest files (didn't do much homework here).

Suggest: You can import and except ScanError instead of having to do all three framework exceptions (QuitScanException, GlobalScanTimeoutError, GlobalModuleTimeoutError). These exceptions inherit ScanError, so ScanError handles them all. This was an addition after several modules were developed, betting you just copy-pasted from one of these old ones (as you should, don't reinvent the wheel).

Suggest: We have tried to stick with 4 space indentations. I noticed the scan_module line in the Yara dispatch rule uses a tab.

jshlbrd commented 8 years ago

Good feedback-- I'll update this soon. Thanks!

jshlbrd commented 8 years ago

All updated-- let me know if you see any other issues. Thanks!

kglm commented 8 years ago

Looks good. I must have overlooked the fact that the try-except is superfluous here since there isn't any exceptional cases, but no harm is done really. Feel free to remove it if you want, if not we will merge it in after a few days.

jshlbrd commented 8 years ago

Fixed, thanks for the clarification.