added MobSF_Service analyzer #2584
Conversation
Attached is the screenshot for the MobSF_Service report and below is the full JSON report containing results for static & dynamic analysis.
[job#38_report.json](https://github.com/user-attachments/files/17957663/job.38_report.json)
|
| "supported_filetypes": [ | ||
| "application/vnd.android.package-archive", | ||
| "application/zip", | ||
| ], |
There was a problem hiding this comment.
you can use this list for android:
application/java-archive
application/vnd.android.package-archive
application/x-dex
application/zip
| "health_check_status": True, | ||
| "type": "file", | ||
| "docker_based": False, | ||
| "maximum_tlp": "RED", |
There was a problem hiding this comment.
Even if you should have control of the MOBSF Instance, it is still an external service so by definition this should be AMBER
| get_runtime_dependency_response.raise_for_status() | ||
|
|
||
| stop_dynamic_analysis = requests.post( | ||
| url=self.mobsf_host + "/api/v1/dynamic/stop_analysis", |
There was a problem hiding this comment.
as you suggested, dynamic analysis time is debatable. This is the reason why I think we should add here a sleep and that this should be a parameter of the analyzer. Which default? 20-30 seconds? What do you think about?
There was a problem hiding this comment.
As I commented here, wouldn't just configuring the soft-time limit do the trick. Because if I keep it at default value 60, "SoftTimeLimitExceeded" error is thrown and my job fails. I had to set it to 400 in order for my job to be successful. Let me know what you think!
There was a problem hiding this comment.
I noticed that no sleep was added so basically the analysis has a really brief duration time, just the time needed to execute all the steps between the calls "start" and "stop". As mentioned, I would add a sleep between the "start" and the later calls, to allow time for the stuff to start properly and the code loaded and maybe to allow the malware to do some nasty things too.
| "default_hooks": "api_monitor,ssl_pinning_bypass,root_bypass,debugger_check_bypass", | ||
| "auxiliary_hooks": "", | ||
| "frida_code": "", |
There was a problem hiding this comment.
these 3 values should be parameters of the analyzer. In that way the users can choose how to leverage dynamic analysis as they wish
|
Hi @mlodic I've made the changes as per your comments. I hope we are good to go, let me know otherwise. :) |
| url=scan_url, data=data, headers=headers, timeout=self.timeout | ||
| ) | ||
| scan_response.raise_for_status() | ||
| logger.info("Static analysis completed successfully") |
There was a problem hiding this comment.
I am bothering about logs because they are really critical in terms of maintenance and debugging so I have other tweaks to ask you :). Here, there is a log without any dynamic variable. It would be very difficult to use in practice because you cannot distinguish it between other logs from other runs. Always add a reference to the file hash.
| scan_response.raise_for_status() | ||
| logger.info("Static analysis completed successfully") | ||
|
|
||
| logger.info("Generating JSON Report for static analysis") |
There was a problem hiding this comment.
you can merge this log with the previous one because they are one next to other so they make sense to be the same log
|
@mlodic The PR is ready for review :) |
There was a problem hiding this comment.
I have to ask you other stuff to have this PR accepted. Some standards needs to be followed:
- a PR against the documentation repository has been added? Please do that following the checklist and add a link to that PR here.
- the static_analysis and dynamic_analysis functions are really difficult to read and they contain a lot or redundant code. I ask you to refactor that code in a way that there is an helper method that executes all of the requests stuff
| "name": "MobSF_Service", | ||
| "description": "[MobSF_service](https://github.com/MobSF/Mobile-Security-Framework-MobSF) can be used for a variety of use cases such as mobile application security, penetration testing, malware analysis, and privacy analysis.", | ||
| "disabled": False, | ||
| "soft_time_limit": 400, |
There was a problem hiding this comment.
as mentioned in a previous comment, you can extend it to 1000 to allow for broader use cases
| get_runtime_dependency_response.raise_for_status() | ||
|
|
||
| stop_dynamic_analysis = requests.post( | ||
| url=self.mobsf_host + "/api/v1/dynamic/stop_analysis", |
There was a problem hiding this comment.
I noticed that no sleep was added so basically the analysis has a really brief duration time, just the time needed to execute all the steps between the calls "start" and "stop". As mentioned, I would add a sleep between the "start" and the later calls, to allow time for the stuff to start properly and the code loaded and maybe to allow the malware to do some nasty things too.
|
thank you, we are ready to merge it! |
2 participants
Closes #2496
Description
Added MobSF_Service analyzer
Type of change
Please delete options that are not relevant.
Checklist
developdumpplugincommand and added it in the project as a data migration. ("How to share a plugin with the community")test_files.zipand you added the default tests for that mimetype in test_classes.py.FREE_TO_USE_ANALYZERSplaybook by following this guide.urlthat contains this information. This is required for Health Checks._monkeypatch()was used in its class to apply the necessary decorators.MockUpResponseof the_monkeypatch()method. This serves us to provide a valid sample for testing.Black,Flake,Isort) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.testsfolder). All the tests (new and old ones) gave 0 errors.DeepSource,Django Doctorsor other third-party linters have triggered any alerts during the CI checks, I have solved those alerts.Important Rules