Skip to content

Conversation

mstfbl
Copy link
Contributor

@mstfbl mstfbl commented Apr 5, 2021

This PR fixes an issue pointed out by Bandit w.r.t. using the xml library to parse untrusted data, by using defusedxml instead.

Bandit output:

>> Issue: [B314:blacklist] Using xml.etree.ElementTree.parse to parse untrusted XML data is known to be vulnerable to XML attacks. Replace xml.etree.ElementTree.parse with its defusedxml equivalent function or make sure defusedxml.defuse_stdlib() is called
   Severity: Medium   Confidence: High
   Location: ./torchvision/datasets/voc.py:206
   More Info: https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html#b313-b320-xml-bad-elementtree

@mstfbl mstfbl force-pushed the mstfbl/add-defusedxml branch from 84df7f4 to 5f75e06 Compare April 6, 2021 00:06
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

type check is failing, can you fix it so that we can merge the PR?

@fmassa
Copy link
Member

fmassa commented Apr 7, 2021

One possible solution to the type checking is to disable it for defusedxml, as was done for some other libraries like

vision/mypy.ini

Lines 63 to 65 in 9e5edbd

[mypy-av.*]
ignore_missing_imports = True

@mstfbl
Copy link
Contributor Author

mstfbl commented Apr 7, 2021

Thank you @fmassa for your reviews and for merging the previous PRs. I have added the mypy type check disable for the defusedxml package.

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

Thanks!

@fmassa fmassa merged commit 7f4ae8c into pytorch:master Apr 8, 2021
facebook-github-bot pushed a commit that referenced this pull request Apr 13, 2021
Summary:
* Added defusedxml to parse untrusted XML data

* Added typecheck disable for defusedxml

Reviewed By: NicolasHug

Differential Revision: D27706948

fbshipit-source-id: 4334745d939c83e763ea5508b6284275c5c7bc32

Co-authored-by: Nicolas Hug <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants