-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[try] fix possible XXE vulnerabilities #10193
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
base: master
Are you sure you want to change the base?
Conversation
The original code may have had an XXE vulnerability, which is now fixed
The original code may have had an XXE vulnerability, which is now largely fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR aims to mitigate potential XXE vulnerabilities in the XML parsing by switching from the standard ElementTree parser to defusedxml's secure parse function.
- Replace the usage of etree.parse with defusedxml.ElementTree.parse.
- Introduce additional imports to handle the revised parsing and exception handling.
# tree = etree.parse('template_vs2005.vcproj') | ||
tree = parse('template_vs2005.vcproj', forbid_dtd=False, forbid_external=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commented-out legacy parsing code may lead to confusion; consider removing it if it's no longer needed.
# tree = etree.parse('template_vs2005.vcproj') | |
tree = parse('template_vs2005.vcproj', forbid_dtd=False, forbid_external=True) | |
Copilot uses AI. Check for mistakes.
@@ -86,7 +88,8 @@ def VS_AddHeadFilesGroup(program, elem, project_path): | |||
def VSProject(target, script, program): | |||
project_path = os.path.dirname(os.path.abspath(target)) | |||
|
|||
tree = etree.parse('template_vs2005.vcproj') | |||
# tree = etree.parse('template_vs2005.vcproj') | |||
tree = parse('template_vs2005.vcproj', forbid_dtd=False, forbid_external=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current configuration allows DTDs (forbid_dtd=False) which may still enable XXE attacks; if the intent is to fully mitigate XXE vulnerabilities, consider setting forbid_dtd to True.
tree = parse('template_vs2005.vcproj', forbid_dtd=False, forbid_external=True) | |
tree = parse('template_vs2005.vcproj', forbid_dtd=True, forbid_external=True) |
Copilot uses AI. Check for mistakes.
拉取/合并请求描述:(PR description)
[
为什么提交这份PR (why to submit this PR)
The original code is subject to XXE attacks.
你的解决方案是什么 (what is your solution)
Add:
from defusedxml.ElementTree import parse
from defusedxml.common import DefusedXmlException
Modified to:
tree = parse('template_vs2005.vcproj', forbid_dtd=False, forbid_external=True)
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality:
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0
代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up