8000 Multiple suppressions on the same rule_id only removes one instance · Issue #104 · MobSF/mobsfscan · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Multiple suppressions on the same rule_id only removes one instance #104

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.

Sign up for GitHub

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

Jump to bottom
Open
mattmook opened this issue Jan 3, 2025 · 1 comment
Open

Multiple suppressions on the same rule_id only removes one instance #104

mattmook opened this issue Jan 3, 2025 · 1 comment

Comments

@mattmook
Copy link
mattmook commented Jan 3, 2025 •
edited
Loading

For a working example, see: https://github.com/appmattus/certificatetransparency/tree/mobsf
There are three files which trigger android_kotlin_logging, each suppressed with a comment, however, only one suppression works. Raised issues https://github.com/appmattus/certificatetransparency/security/code-scanning?query=pr%3A127+is%3Aopen

Looking at the code there's looks like there's a bug in post_ignore_files, where we remove matches from the original list of files rather than the filtered tmp_files:

i.e.

                if self.suppress_pm_comments(file, rule_id):
                    # remove all matches of the file for the rule
                    tmp_files = self.remove_matches(file, files)

should be

                if self.suppress_pm_comments(file, rule_id):
                    # remove all matches of the file for the rule
                    tmp_files = self.remove_matches(file, tmp_files)

Adding an additional file in tests/assets/src/dot_mobsf with the same existing comment suppression shows this failure in unit tests. For example I duplicated scan_but_ignore.kt as scan_but_ignore2.kt, which results in a failed test.

With the above change to post_ignore_files the test then passes as expected.
mattmook added a commit to mattmook/mobsfscan that referenced this issue Jan 3, 2025
@mattmook
Ensure multiple suppressions work as expected
f2afe9c 
Fixes issue MobSF#104
This was referenced Jan 3, 2025
Open
Merged
@ajinabraham
Copy link
Member

Thanks for the detailed report. I will take a look at this and get back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mattmook @ajinabraham
0