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

Performance improvement in blacklist function #1148

Merged
merged 2 commits into from
Jun 24, 2024
Merged

Conversation

ericwb
Copy link
Member

@ericwb ericwb commented Jun 23, 2024

The blacklisting function is currently using fnmatch.fnmatch() to do matching of qualified names of blacklist calls. It seems it is only used for telnetlib and ftplib where they are setting the qualified name in a file glob style (telnetlib.*).

This change would slightly break backward compatibility if there are any third-party plugins that use globbing in the qualified names for blacklisting. I think the likelyhood is small. I also think it is better to be more explicit in the qualified name patterns. In the case of ftplib, FTP is insecure, but FTP_TLS is not. So this already is resolving one false postive.

The other effect of this change is a slight boost to performance. When scanning cpython prior to this fix, it would take around 1 min. After the fix, closer to 50 seconds. So a nice little bump in speed.

Fixes: #438

@ericwb
Copy link
Member Author

ericwb commented Jun 23, 2024

Before fix (scanning cpython):

real	1m2.749s
user	1m1.246s
sys	0m0.442s

After fix:

real	0m50.999s
user	0m49.539s
sys	0m0.388s

The blacklisting function is currently using fnmatch.fnmatch()
to do matching of qualified names of blacklist calls. It seems
it is only used for telnetlib and ftplib where they are setting
the qualified name in a file glob style (telnetlib.*).

This change would slightly break backward compatibility if there
are any third-party plugins that use globbing in the qualified
names for blacklisting. I think the likelyhood is small. I also
think it is better to be more explicit in the qualified name
patterns. In the case of ftplib, FTP is insecure, but FTP_TLS is
not. So this already is resolving one false postive.

The other effect of this change is a slight boost to performance.
When scanning cpython prior to this fix, it would take around 1 min.
After the fix, closer to 50 seconds. So a nice little bump in speed.

Fixes: PyCQA#438

Signed-off-by: Eric Brown <[email protected]>
ericwb added a commit to ericwb/bandit that referenced this pull request Jun 23, 2024
This change adds an FTP_TLS call to the examples. A high severity
error is no longer reported as a result of the fix in PR PyCQA#1148
that explicitly now matches blacklist call qualified names rather
than using a file glob.

However, you will notice that there is one more high severity
issue reported in the tests as a result of the import of
ftplib.FTP_TLS because the blacklist import is only checking for
"ftplib".

Fixes: PyCQA#148

Signed-off-by: Eric Brown <[email protected]>
@ericwb
Copy link
Member Author

ericwb commented Jun 24, 2024

Merging, as this is a dependency for approved PR #1149

@ericwb ericwb merged commit 4208e9d into PyCQA:main Jun 24, 2024
16 checks passed
@ericwb ericwb deleted the perf branch June 24, 2024 00:07
ericwb added a commit that referenced this pull request Jun 24, 2024
* Performance improvement in blacklist function

The blacklisting function is currently using fnmatch.fnmatch()
to do matching of qualified names of blacklist calls. It seems
it is only used for telnetlib and ftplib where they are setting
the qualified name in a file glob style (telnetlib.*).

This change would slightly break backward compatibility if there
are any third-party plugins that use globbing in the qualified
names for blacklisting. I think the likelyhood is small. I also
think it is better to be more explicit in the qualified name
patterns. In the case of ftplib, FTP is insecure, but FTP_TLS is
not. So this already is resolving one false postive.

The other effect of this change is a slight boost to performance.
When scanning cpython prior to this fix, it would take around 1 min.
After the fix, closer to 50 seconds. So a nice little bump in speed.

Fixes: #438

Signed-off-by: Eric Brown <[email protected]>

* Add test for usage of FTP_TLS

This change adds an FTP_TLS call to the examples. A high severity
error is no longer reported as a result of the fix in PR #1148
that explicitly now matches blacklist call qualified names rather
than using a file glob.

However, you will notice that there is one more high severity
issue reported in the tests as a result of the import of
ftplib.FTP_TLS because the blacklist import is only checking for
"ftplib".

Fixes: #148

Signed-off-by: Eric Brown <[email protected]>

---------

Signed-off-by: Eric Brown <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Massive reduction in performance after 1.5.0
1 participant