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.

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

update B405 rules #1120

Closed
wants to merge 1 commit into from
Closed

update B405 rules #1120

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions bandit/blacklists/imports.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@
| ID | Name | Imports | Severity |
+======+=====================+====================================+===========+
| B405 | import_xml_etree | - xml.etree.cElementTree | low |
| | | - xml.etree.ElementTree | |
| | | - xml.etree.ElementTree.XMLParser | |
| | | - xml.etree.ElementTree.fromstring | |
| | | - xml.etree.ElementTree.iterparse | |
| | | - xml.etree.ElementTree.parse | |
+------+---------------------+------------------------------------+-----------+

B406: import_xml_sax
Expand Down Expand Up @@ -308,7 +311,13 @@ def gen_blacklist():
"import_xml_etree",
"B405",
issue.Cwe.IMPROPER_INPUT_VALIDATION,
["xml.etree.cElementTree", "xml.etree.ElementTree"],
[
"xml.etree.cElementTree",
"xml.etree.ElementTree.XMLParser",
"xml.etree.ElementTree.fromstring",
"xml.etree.ElementTree.iterparse",
"xml.etree.ElementTree.parse",
Comment on lines +317 to +319
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe you can import fromstring, iterparse, or parse as a result this doesn't do what you want.

Copy link
Author

@kiraware kiraware Mar 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Ah sorry, looks like B314 rules already cover it. dumb me.

Copy link
Author

@kiraware kiraware Mar 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What do you think, because B405 rules are too general and make the entire contents of the ElementTree module seem dangerous from a security perspective even though there are functions or classes that are not dangerous, for example Element and that is needed for typing. Do I need to add all the functions or classes that are dangerous and not include those that have nothing to do with security as I have listed above?

Before the changes in this PR, this code will warn B405 rules. But after changes in this PR, this code will not warn B405.

import xml.etree.ElementTree

And after changes in this PR too, this code will warn B405, although bandit only warn the fromstring and the rest is not. Looks weird.

from xml.etree.ElementTree import fromstring, parse, iterparse, XMLParser

If there was a way for bandit to raise a warning to a module but some of the contents of that module didn't raise a warning things will become simple and and I don't have to give # nosec: B405 everywhere.

I don't believe you can import fromstring, iterparse, or parse as a result this doesn't do what you want.

This code will result in ModuleNotFoundError.

import xml.etree.ElementTree.fromstring
import xml.etree.ElementTree.parse
import xml.etree.ElementTree.iterparse
import xml.etree.ElementTree.XMLParser

instead someone will use this

from xml.etree.ElementTree import fromstring, parse, iterparse, XMLParser

],
xml_msg,
"LOW",
)
Expand Down
Loading