Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor mobsf parser for v4 reports #11056

Merged
merged 33 commits into from
Nov 13, 2024

Conversation

dmarushkin
Copy link
Contributor

@dmarushkin dmarushkin commented Oct 12, 2024

Hi guys!

Looks like in 4th version of Mobsf json reports parser failed again for android and ios, it loose some vulns after loading and has html tags in vuln titles and descriptions.

I've drilled down into current reports structure and parser logic and found that new reports in Mobsf have appsec section with same structure for ios and android reports and well rendered by maintainers vuln titles and descriptions, in mobsf UI it used in Scoreboad:

image

So I deside to refactor current parser to get dd findings and test description from this appsec items.

Results look like:

image

image

image

image

image

image

image

Copy link

dryrunsecurity bot commented Oct 12, 2024

DryRun Security Summary

The provided code changes enhance the DefectDojo application's integration and parsing of the MobSF (Mobile Security Framework) Scorecard tool, including the addition of a new parser, configuration updates, documentation, and unit tests to ensure accurate representation and prioritization of security issues in mobile applications.

Expand for full summary

Summary:

The provided code changes introduce several improvements and additions to the DefectDojo application, primarily focused on enhancing the integration and parsing of the MobSF (Mobile Security Framework) Scorecard tool. These changes include:

  1. MobSF Scorecard Parser: The addition of a new parser, MobSFScorecardParser, which is responsible for parsing the JSON output of the MobSF Scorecard tool and mapping the identified security findings to the DefectDojo Finding model. This ensures accurate representation and prioritization of security issues in mobile applications.

  2. Configuration Updates: The changes to the dojo/settings/.settings.dist.py.sha256sum and dojo/settings/settings.dist.py files update the configuration to support the new MobSF Scorecard parser. This includes mapping the parser to the appropriate deduplication algorithm and SAML2 attribute mapping format.

  3. Documentation: The new documentation file docs/content/en/integrations/parsers/file/mobsf_scorecard.md provides information on how to export a JSON file using the MobSF Scorecard API endpoint and includes a link to sample scan data.

  4. Unit Tests: The addition of the unittests/tools/test_mobsf_scorecard_parser.py file introduces a comprehensive suite of unit tests for the MobSFScorecardParser class, ensuring the parser's reliability and robustness in handling various input scenarios.

Overall, these changes enhance the DefectDojo application's capabilities in managing and tracking security vulnerabilities in mobile applications by integrating the MobSF Scorecard tool. The code changes appear to be well-designed and secure, with a focus on accurate parsing, severity mapping, and deduplication of security findings.

Files Changed:

  1. dojo/tools/mobsf_scorecard/__init__.py: This file has a minor change, adding an __author__ attribute to the module.
  2. docs/content/en/integrations/parsers/file/mobsf_scorecard.md: This new documentation file provides information on the MobSF Scorecard Scanner integration.
  3. dojo/settings/.settings.dist.py.sha256sum: The SHA-256 hash value in this file has been updated, indicating a change to the corresponding .settings.dist.py configuration file.
  4. dojo/settings/settings.dist.py: This file has been updated to add the "MobSF Scorecard Scan" parser to the saml2_attrib_map_format and DEDUPLICATION_ALGORITHM_PER_PARSER dictionaries.
  5. dojo/tools/mobsf_scorecard/parser.py: This new file contains the implementation of the MobSFScorecardParser class, which is responsible for parsing the MobSF Scorecard report and mapping the findings to the DefectDojo Finding model.
  6. unittests/tools/test_mobsf_scorecard_parser.py: This new file contains a comprehensive suite of unit tests for the MobSFScorecardParser class, ensuring the reliability and robustness of the parser.

Code Analysis

We ran 9 analyzers against 12 files and 0 analyzers had findings. 9 analyzers had no findings.

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

@Maffooch
Copy link
Contributor

Does this PR maintain backward compatibility with older versions of the tool?

@dmarushkin
Copy link
Contributor Author

@Maffooch Hi Cody!
Looks like Scoreboard and appsec section in report exists from Jan 2022 and Mobsf version 3.5 (link with change)

@Maffooch
Copy link
Contributor

Sorry, I should have been more specific. Is the MobSF parser compatible with older versions of the tool after the changes in this PR? It looks like all of the unit tests and sample scans are removed/rewritten in this PR. If this change to support version 4.x of MobSF cannot support older formats of MobSF, then the PR cannot be accepted

@dmarushkin
Copy link
Contributor Author

@Maffooch Yes, reports from tool with version older then 3.5 will not be loaded in new parser at all, reports with version from 3.5+ will be loaded with different results from new parser and old one, cause new parser use agregated results from maintainers of mobsf, old one parses raw data from report sections (and has bugs with that).

I can add switch, if version of report > 4 use new logic, else use old one.

As alternative I can leave current parser as is and add new one with name Mobsf Scorecard, so change will not break logic for exist users and add alternative for new setups.

@Maffooch
Copy link
Contributor

If the results are significantly different than the older format, then a new parser might be best. I'll leave that up to you to decide 😄

The main point is that we can maintain to the old and the new formats at once

@github-actions github-actions bot added settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR docs labels Oct 27, 2024
@dmarushkin
Copy link
Contributor Author

dmarushkin commented Oct 27, 2024

@Maffooch Hi Cody! Sorry for long delay... I've separate all new logic into parser MobSF Scorecard Scan and restore old parser files as is.

Hope this fits )

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@dmarushkin
Copy link
Contributor Author

@mtesauro Yeh, now messed up with paths to sample reports and lost test for old parser...
fixed, hope that settings hash in dev has not changed again )

Copy link
Contributor

github-actions bot commented Nov 4, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@mtesauro
Copy link
Contributor

mtesauro commented Nov 5, 2024

@dmarushkin Yikes. Looks like that hash has changed - I went ahead and kicked off the tests again. Wait till they finish to fix the sha256sum and we should be good to start the reviews & approvals.

Thanks for your patience on this one 👍

Copy link
Contributor

github-actions bot commented Nov 7, 2024

Conflicts have been resolved. A maintainer will review the pull request shortly.

@dmarushkin
Copy link
Contributor Author

dmarushkin commented Nov 7, 2024

@mtesauro Fixed hash, double-checked that new one will actual for new dev settings with applied change.

dojo/fixtures/defect_dojo_sample_data.json Outdated Show resolved Hide resolved
unittests/tools/test_mobsf_scorecard_parser.py Outdated Show resolved Hide resolved
dojo/tools/mobsf_scorecard/parser.py Outdated Show resolved Hide resolved
Copy link
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@mtesauro
Copy link
Contributor

@dmarushkin Sorry, one more fix of the sha256sum and I'll quickly merge this before it happens again.

BTW, we're looking at different methods for what that sum does so this doesn't impact contributors so sorry you've had to deal with it so much in the PR. Hopefully, for your next contribution, it will be gone. 🤞

Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@dmarushkin
Copy link
Contributor Author

dmarushkin commented Nov 13, 2024

@mtesauro Not a problem, then God was giving others brain, I was in queue for patience )

Dmitry Maryushkin added 2 commits November 13, 2024 21:18
@Maffooch Maffooch merged commit c3c3d92 into DefectDojo:dev Nov 13, 2024
73 checks passed
@dmarushkin dmarushkin deleted the fix_parser_for_mobsf4 branch November 13, 2024 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs parser settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR unittests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants