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

Increase robustness of XMLDocument parser #1515

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

AMS21
Copy link
Contributor

@AMS21 AMS21 commented Nov 11, 2023

  • Initialize m_bIgnoreMissingEndTagError
  • Pass down bool fatal to ParseFile. Previously we would still hit assertions for invalid #includes with fatal set to false
  • Fix potential use of nullptr when strstr returns nullptr
  • Introduce a non failing variant of r_string called try_r_string
  • Fix various problems with the handling of #include, remove not needed string copy and give feedback on invalid things instead of silently ignoring problems.
  • Fix an infinite loop in tinyxml when parsing an invalid UTF-8 sequence
  • Fix infinite recursion or cyclic includes causing a stack overflow

src/xrCore/XML/XMLDocument.cpp Show resolved Hide resolved
src/xrCore/XML/XMLDocument.cpp Outdated Show resolved Hide resolved
src/xrCore/XML/XMLDocument.cpp Outdated Show resolved Hide resolved
@AMS21
Copy link
Contributor Author

AMS21 commented Nov 11, 2023

Implemented suggested changes

@AMS21 AMS21 force-pushed the parser/xml_document branch 3 times, most recently from 5c1b613 to 8487911 Compare November 12, 2023 13:02
@AMS21
Copy link
Contributor Author

AMS21 commented Nov 12, 2023

Left the fuzzer I wrote running last night and it found another problem with a file including itself causing a stack overflow

- Initialize `m_bIgnoreMissingEndTagError`
- Pass down `bool fatal` to `ParseFile`. Previously we would still hit
  assertions for invalid `#includes` with `fatal` set to `false`
- Fix potential use of nullptr when `strstr` returns `nullptr`
- Introduce a non failing variant of `r_string` called `try_r_string`
- Fix various problems with the handling of `#include`, remove not needed
  string copy and give feedback on invalid things instead of silently
  ignoring problems.
- Fix an infinite loop in tinyxml when parsing an invalid UTF-8 sequence
- Fix infinite recursion or cyclic includes causing a stack overflow
@Xottab-DUTY Xottab-DUTY merged commit 403fb59 into OpenXRay:dev Nov 17, 2023
21 of 22 checks passed
@AMS21 AMS21 deleted the parser/xml_document branch November 17, 2023 18:28
@AMS21 AMS21 restored the parser/xml_document branch November 21, 2023 15:00
@AMS21 AMS21 deleted the parser/xml_document branch November 21, 2023 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Quality Enhancement Modmaker Experience Modmaker experience with OpenXRay
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants