mobsf

[g4ze]

closes #2248

Description

Please include a summary of the change and link to the related issue.

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue).
• New feature (non-breaking change which adds functionality).
• Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist

• I have read and understood the rules about how to Contribute to this project
• The pull request is for the branch develop
• A new plugin (analyzer, connector, visualizer, playbook, pivot or ingestor) was added or changed, in which case:
  • I strictly followed the documentation "How to create a Plugin"
  • Usage file was updated.
  • Advanced-Usage was updated (in case the plugin provides additional optional configuration).
  • I have dumped the configuration from Django Admin using the dumpplugin command and added it in the project as a data migration. ("How to share a plugin with the community")
  • If a File analyzer was added and it supports a mimetype which is not already supported, you added a sample of that type inside the archive test_files.zip and you added the default tests for that mimetype in test_classes.py.
  • If you created a new analyzer and it is free (does not require any API key), please add it in the FREE_TO_USE_ANALYZERS playbook by following this guide.
  • Check if it could make sense to add that analyzer/connector to other freely available playbooks.
  • I have provided the resulting raw JSON of a finished analysis and a screenshot of the results.
  • If the plugin interacts with an external service, I have created an attribute called precisely url that contains this information. This is required for Health Checks.
  • If the plugin requires mocked testing, _monkeypatch() was used in its class to apply the necessary decorators.
  • I have added that raw JSON sample to the MockUpResponse of the _monkeypatch() method. This serves us to provide a valid sample for testing.
• If external libraries/packages with restrictive licenses were used, they were added in the Legal Notice section.
• Linters (Black, Flake, Isort) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.
• I have added tests for the feature/bug I solved (see tests folder). All the tests (new and old ones) gave 0 errors.
• If changes were made to an existing model/serializer/view, the docs were updated and regenerated (check CONTRIBUTE.md).
• If the GUI has been modified:
  • I have a provided a screenshot of the result in the PR.
  • I have created new frontend tests for the new component or updated existing ones.
• After you had submitted the PR, if DeepSource, Django Doctors or other third-party linters have triggered any alerts during the CI checks, I have solved those alerts.

Important Rules

• If you miss to compile the Checklist properly, your PR won't be reviewed by the maintainers.
• Everytime you make changes to the PR and you think the work is done, you should explicitly ask for a review. After being reviewed and received a "change request", you should explicitly ask for a review again once you have made the requested changes.

[g4ze]

hey! @mlodic There is some dependency conflict for mobsf dependencies:
ERROR: Cannot install -r project-requirements.txt (line 17), geoip2, libsast and mobsfscan because these package versions have conflicting dependencies.
167.7
167.7 The conflict is caused by:
167.7 jsonschema 4.23.0 depends on attrs>=22.2.0
167.7 aiohttp 3.6.2 depends on attrs>=17.3.0
167.7 jschema-to-python 1.2.3 depends on attrs
167.7 semgrep 0.117.0 depends on attrs~=21.3

should i intsall newer versions?

[mlodic]

oh nou, dependency conflicts...

I think it makes sense to add this tool in the "malware_tools_analyzers" too with its own python environment. That's the only way to avoid becoming mad

[g4ze]

agreed 😆

[g4ze]

{
                "errors": [],
                "results": {
                    "android_logging": {
                        "files": [
                            {
                                "file_path": "/tmp/tmpzpwdglcv/java_vuln.java",
                                "match_lines": [
                                    19,
                                    19
                                ],
                                "match_string": "            Log.d(\"htbridge\", \"getAllRecords(): \" + records.toString());",
                                "match_position": [
                                    13,
                                    73
                                ]
                            }
                        ],
                        "metadata": {
                            "cwe": "CWE-532: Insertion of Sensitive Information into Log File",
                            "masvs": "MSTG-STORAGE-3",
                            "severity": "INFO",
                            "reference": "https://github.com/MobSF/owasp-mstg/blob/master/Document/0x05d-Testing-Data-Storage.md#logs",
                            "description": "The App logs information. Please ensure that sensitive information is never logged.",
                            "owasp-mobile": "M1: Improper Platform Usage"
                        }
                    },
                    "android_safetynet_api": {
                        "metadata": {
                            "cwe": "CWE-353: Missing Support for Integrity Check",
                            "masvs": "MSTG-RESILIENCE-1",
                            "severity": "INFO",
                            "reference": "https://github.com/MobSF/owasp-mstg/blob/master/Document/0x05j-Testing-Resiliency-Against-Reverse-Engineering.md#testing-root-detection-mstg-resilience-1",
                            "description": "This app does not uses SafetyNet Attestation API that provides cryptographically-signed attestation, assessing the device's integrity. This check helps to ensure that the servers are interacting with the genuine app running on a genuine Android device. ",
                            "owasp-mobile": "M8: Code Tampering"
                        }
                    },
                    "android_root_detection": {
                        "metadata": {
                            "cwe": "CWE-919: Weaknesses in Mobile Applications",
                            "masvs": "MSTG-RESILIENCE-1",
                            "severity": "INFO",
                            "reference": "https://github.com/MobSF/owasp-mstg/blob/master/Document/0x05j-Testing-Resiliency-Against-Reverse-Engineering.md#testing-root-detection-mstg-resilience-1",
                            "description": "This app does not have root detection capabilities. Running a sensitive application on a rooted device questions the device integrity and affects users data.",
                            "owasp-mobile": "M8: Code Tampering"
                        }
                    },
                    "android_detect_tapjacking": {
                        "metadata": {
                            "cwe": "CWE-200: Information Exposure",
                            "masvs": "MSTG-PLATFORM-9",
                            "severity": "INFO",
                            "reference": "https://github.com/MobSF/owasp-mstg/blob/master/Document/0x05h-Testing-Platform-Interaction.md#testing-for-overlay-attacks-mstg-platform-9",
                            "description": "This app does not have capabilities to prevent tapjacking attacks. An attacker can hijack the user's taps and tricks him into performing some critical operations that he did not intend to.",
                            "owasp-mobile": "M1: Improper Platform Usage"
                        }
                    },
                    "android_prevent_screenshot": {
                        "metadata": {
                            "cwe": "CWE-200: Information Exposure",
                            "masvs": "MSTG-STORAGE-9",
                            "severity": "INFO",
                            "reference": "https://github.com/MobSF/owasp-mstg/blob/master/Document/0x05d-Testing-Data-Storage.md#finding-sensitive-information-in-auto-generated-screenshots-mstg-storage-9",
                            "description": "This app does not have capabilities to prevent against Screenshots from Recent Task History/ Now On Tap etc.",
                            "owasp-mobile": "M2: Insecure Data Storage"
                        }
                    },
                    "android_certificate_pinning": {
                        "metadata": {
                            "cwe": "CWE-295: Improper Certificate Validation",
                            "masvs": "MSTG-NETWORK-4",
                            "severity": "INFO",
                            "reference": "https://github.com/MobSF/owasp-mstg/blob/master/Document/0x05g-Testing-Network-Communication.md#testing-custom-certificate-stores-and-certificate-pinning-mstg-network-4",
                            "description": "This app does not use a TLS/SSL certificate or public key pinning in code to detect or prevent MITM attacks in secure communication channel. Please verify if pinning is enabled in `network_security_config.xml`.",
                            "owasp-mobile": "M3: Insecure Communication"
                        }
                    }
                },
                "mobsfscan_version": "0.3.9"
            },
            

[g4ze]

is there any new procedure to update the usage doc file?

[g4ze]

the test fails here is due to this last commit made in the develop branch.

[mlodic]

about the doc file, now you need to change it here: https://github.com/intelowlproject/docs/blob/main/docs/IntelOwl/usage.md I'll update the PR template

[mlodic]

the test fails here is due to this last commit made in the develop branch.

That was a mistake of mine. You can readd that package

[mlodic]

once fixed those, the PR is gtg

[g4ze]

what strategy should we opt to update the usage file? can i make a one separate branch with the usage file updated for all the 4 analyzers in process. That way we could just merge it once while updating it with the develop branch.

[mlodic]

yes, you can create a PR request in that docs repository, just once

[mlodic]

I think tests are failing cause in the "normal" CI (see here ) we don't execute the malware tools container because it's too heavy.

It would be cool to find some way to have those tests executed if and only when that container is up.

[g4ze]

So we need docker analyser tests to run only when the particular container is up? If that's the case then how did the prev docker based analyzers that I implemented pass the tests? Plus we are already mocking the result so it shouldn't be dependent on the container response? Can you please lmk if I'm missing something here in the picture?

[mlodic]

So we need docker analyser tests to run only when the particular container is up?

no you are right, I checked again and this is not necessary

If that's the case then how did the prev docker based analyzers that I implemented pass the tests?

can you bring an example? even that time you added a new mimetype?

Plus we are already mocking the result so it shouldn't be dependent on the container response?

Yes

In the Java example that you tried manually, is the file correctly tagged as text/x-java?

[g4ze]

#2461 (comment)

I've pasted a run here. By tag you mean that the file is identified correctly right?

An analyser with similar case can be #2401 that was implemented a while ago. It's not exactly the same but can be related.

[g4ze]

Any ideas for this? @mlodic

[mlodic]

looking at the other goresym analyzer, the only things that changes is that you created 2 different migrations instead of a single one. You can try by adding first a migration for the filetype only and then the analyzer and see if anything changes.
Otherwise the filetype, have you tried to run the same test locally? In that way you can start to add some prints and tests here and there to understand the root issue

[g4ze]

it was a problem with the mockup response 😿

[g4ze]

i think we are gtg here @mlodic

[mlodic]

one thing missing: you commented some code while testing, can you remove it?
plus, migrations order, let's do that in this order:

• mobsf
• droidlys
• apk artifacts
  and then we are done

[mlodic]

ah and remember to update this file: https://github.com/intelowlproject/docs/blob/main/docs/IntelOwl/usage.md

thank you!

[mlodic]

another important thing. The test_files.zip. The droidlys PR should have all the mobsf files too otherwise we would merge a broken PR

[g4ze]

all done

[mlodic]

great! merged!