Skip to content

Initialize m_dtd member in ValidateDTD class as NULL #1751

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

Closed
wants to merge 1 commit into from

Conversation

airween
Copy link
Member

@airween airween commented Apr 23, 2018

In case of using GCC (and also libxml2 is used), the m_dtd just declared, but not initialized - GCC initialized it as NON NULL pointer. When destructor called, it checks the m_dtd, and if it's not NULL, calls xmlFreeDtd. The result is a segfault:

airween@vm:~/src/ModSecurity$ test/regression_tests test/test-cases/regression/config-xml_external_entity.json 
ModSecurity 3.0.2 - tests
(options are not available -- missing GetOpt)

  # File Name                                         Test Name                                                             Passed?   
--- ---------                                         ---------                                                             -------   
Segmentation fault

The expected behavior would be:

airween@vm:~/src/ModSecurity$ test/regression_tests test/test-cases/regression/config-xml_external_entity.json 
ModSecurity 3.0.2 - tests
(options are not available -- missing GetOpt)

  # File Name                                         Test Name                                                             Passed?   
--- ---------                                         ---------                                                             -------   
  1 config-xml_external_entity.json                   Testing SecXMLExternalEntity/XXE 1                                    passed!
  2 config-xml_external_entity.json                   Testing SecXMLExternalEntity/XXE 2                                    failed!
  3 config-xml_external_entity.json                   Testing SecXMLExternalEntity/XXE 3                                    failed!

Test failed. From: test/test-cases/regression/config-xml_external_entity.json.
Test name: Testing SecXMLExternalEntity/XXE 2.
Reason: 
parse failed.
Rules error. File: config-xml_external_entity.json. Line: 6. Column: 11. XML: File not found: test-cases/data/SoapEnvelope.dtd. Looking at: 'test-cases/data/SoapEnvelope.dtd', 'test-cases/data/SoapEnvelope.dtd', 'config-xml_external_entity.json/test-cases/data/SoapEnvelope.dtd', 'config-xml_external_entity.json/test-cases/data/SoapEnvelope.dtd'. 

Test failed. From: test/test-cases/regression/config-xml_external_entity.json.
Test name: Testing SecXMLExternalEntity/XXE 3.
Reason: 
parse failed.
Rules error. File: config-xml_external_entity.json. Line: 6. Column: 11. XML: File not found: test-cases/data/SoapEnvelope.dtd. Looking at: 'test-cases/data/SoapEnvelope.dtd', 'test-cases/data/SoapEnvelope.dtd', 'config-xml_external_entity.json/test-cases/data/SoapEnvelope.dtd', 'config-xml_external_entity.json/test-cases/data/SoapEnvelope.dtd'. 

Ran a total of: 3 regression tests - 2 failed. 0 skipped test(s). 0 disabled test(s).

This patch prevents the segfault when the DTD is not found.

Note, that I've started the regression_test from the ROOT of ModSecurity source tree, then this check reproducable. If you go to src/ModSecurity/test, and the command is just "./regression_test test-cases/regression/config-xml_external_entity.json", then the test exits normally with passes of all tests.

Even so, it would be better to avoid the segfault...

Also note, that in case of using CLANG, the problem is not reproducible - perhaps CLANG initializes the member as NULL automatically.

@zimmerle zimmerle self-assigned this Apr 24, 2018
@zimmerle zimmerle added the 3.x Related to ModSecurity version 3.x label Apr 24, 2018
@zimmerle zimmerle added this to the v3.0.3 milestone Apr 24, 2018
@zimmerle
Copy link
Contributor

Hi @airween, good catch! merged. Thanks.

@zimmerle zimmerle closed this Apr 24, 2018
zimmerle pushed a commit that referenced this pull request Apr 24, 2018
@airween airween deleted the v3/xmldtdnullp branch April 24, 2018 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Related to ModSecurity version 3.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants