Replace xml.etree.ElementTree by defusedxml.ElementTree

[wwXing0]

Bandit scan tool reports the following issue: Using various methods to parse untrusted XML data is known to be vulnerable to XML attacks. Should replace vulnerable imports with the equivalent defusedxml package.

[CLAassistant]

All committers have signed the CLA.

[wwXing0]

@jenatali @bashbaug Please help to review. Thanks a lot.

[bashbaug]

I'm slightly torn about this: On the one hand, the only XML we're parsing is within our control and we only parse XML for our code generation scripts. On the other hand, defusedxml seems to be a drop-in replacement for xml.tree, so the drawbacks of switching seem to be minimal (just need to install the defusedxml library).

Several notes if we decide to do this:

1. We should update the README to indicate that defusedxml is required in addition to Mako. @wwXing0 would you mind adding this?
   • Note, we don't have a requirements.txt right now; this would be a nice addition but if we decide to add it we should do so with a separate PR.
2. We should update the other OpenCL repos that parse the XML file for code generation similarly.
   • OpenCL-Docs parses the XML file to generate spec fragments.
   • OpenCL-Headers may eventually parse the XML file to generate headers - https://github.com/KhronosGroup/OpenCL-Headers/pull/161.

[wwXing0]

@bashbaug OK, I will add it.

1. We should update the README to indicate that defusedxml is required in addition to Mako. @wwXing0 would you mind adding this?

Could you please help to add them for other OpenCL repos?

2. We should update the other OpenCL repos that parse the XML file for code generation similarly.

[bashbaug]

Could you please help to add them for other OpenCL repos?

Yes, I can take care of this, thanks!

[wwXing0]

@bashbaug Please take a look, and help to merge. Thanks

[wwXing0]

@bashbaug Could you please help to merge ?

[oddhack]

The API XML we write doesn't actually use any of the vulnerable points identified for xml.ElementTree so the likelihood of encountering problems is minute - someone would have to inject an attack via a github PR and somehow get it past what seems likely to be a failed CI run.

OTOH if defusedxml is truly compatible with the builtin there's probably no strong reason not to use it (although it's not clear to me how actively it's being maintained - last update was in 2021). I did at one point encounter a very, very subtle incompatibility between lxml and xml.ElementTree when I switched between them and it's not impossible that exists here as well - it required some gymnastics when deleting an element from the tree.

This reminds me that I should again sync up the OpenCL-Docs scripts with the much more actively updated ones in OpenXR and Vulkan, to try and keep us all on the same page.

[wwXing0]

Sorry for the late response. @oddhack Sorry, I'm also not sure if there will be compatibility issues after the replacement. @bashbaug Do you think we can do this replacement?

[wwXing0]

It seems that there will be a risk of unknown compatibility when changing to defusedxml and keep using xml.etree.ElementTree won't run into XML attack problems. So won't fix this issue, and close this pr.