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

Fix possible vulnerabilities in the XML calls in framework #10113

Closed
13 tasks done
Tracked by #2330
mcarmona99 opened this issue Sep 14, 2021 · 1 comment · Fixed by #10635
Closed
13 tasks done
Tracked by #2330

Fix possible vulnerabilities in the XML calls in framework #10113

mcarmona99 opened this issue Sep 14, 2021 · 1 comment · Fixed by #10635
Assignees

Comments

@mcarmona99
Copy link
Contributor

mcarmona99 commented Sep 14, 2021

Description

With the test created in the issue wazuh/wazuh-qa#1615, some possible code flaws were found by Bandit.

In this issue we specify flaws regarding the use of insecure XML calls.

Issue 1:

Using tostring to parse untrusted XML data is known to be vulnerable to XML attacks. Replace tostring with the equivalent defusedxml package, or make sure defusedxml.defuse_stdlib() is called.

framework/wazuh/core/configuration.py line 15

from xml.etree.ElementTree import tostring

Issue 2:

Using ElementTree to parse untrusted XML data is known to be vulnerable to XML attacks. Replace ElementTree with the equivalent defusedxml package, or make sure defusedxml.defuse_stdlib() is called.

framework/wazuh/core/utils.py line 25

from xml.etree.ElementTree import ElementTree

As Bandit indicates, in both cases it is convenient to use defusedxml because it does not reduce functionality and increases security.
These flaws can be avoided with the use of defusedxml (we have already used it in other situations, so we have the dep in the requirements.txt).

This issue aims to delete this vulnerabilities by using the defusedxml dep.

More info.: https://bandit.readthedocs.io/en/latest/blacklists/blacklist_imports.html#b405-import-xml-etree

Once changes are done, pass the test to check that these flaws were deleted from the known flaws JSON file of framework.

Checks

wazuh/wazuh

  • Unit tests without failures. Updated and/or expanded if there are new functions/methods/outputs:
    • Cluster (framework/wazuh/core/cluster/tests/ & framework/wazuh/core/cluster/dapi/tests/)
    • Core (framework/wazuh/core/tests/)
    • SDK (framework/wazuh/tests/)
    • RBAC (framework/wazuh/rbac/tests/)
    • API (api/api/tests/)
  • API tavern integration tests without failures. Updated and/or expanded if needed (api/test/integration/):
    • Affected tests
    • Affected RBAC (black and white) tests
  • Review integration test mapping using the script (api/test/integration/mapping/integration_test_api_endpoints.json)
  • Review of spec.yaml examples and schemas (api/api/spec/spec.yaml)
  • Review exceptions remediation when any endpoint path changes or is removed (framework/wazuh/core/exception.py)
  • Changelog (CHANGELOG.md)

wazuh/wazuh-documentation

  • Migration from 3.X for changed endpoints (source/user-manual/api/equivalence.rst)
  • Update RBAC reference with new/modified actions/resources/relationships (source/user-manual/api/rbac/reference.rst)
@Kondent
Copy link
Contributor

Kondent commented Oct 21, 2021

Update

I made adjustments to start using defusedxml package.
After those changes I did few manual tests and AIT to check its properly functioning.

I found out that there are some references of xml.etree package inside unit tests and one AIT tool.

↪ ~/git/wazuh ⊶ fix/10113-framework-xml-calls ⨘ rgrep 'xml.etree' | grep -v 'Binary'
api/test/integration/env/tools/xml_parser.py:from xml.etree.ElementTree import parse
framework/wazuh/core/tests/test_configuration.py:from xml.etree.ElementTree import fromstring
framework/wazuh/core/tests/test_utils.py:from xml.etree.ElementTree import Element, parse

To be consistent with this development, it would be good to have these references fixed too. I'm currently working on them.

Regards,
Alexis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants