-
Notifications
You must be signed in to change notification settings - Fork 166
Fix invalid XML Buffer Overflow Error #1201
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
Conversation
distributionBase=GRADLE_USER_HOME | ||
distributionPath=wrapper/dists | ||
distributionUrl=https\://services.gradle.org/distributions/gradle-8.9-bin.zip |
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.
VSCode promoted me to upgrade Gradle.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1201 +/- ##
==========================================
- Coverage 84.25% 84.25% -0.01%
==========================================
Files 57 57
Lines 5984 5989 +5
==========================================
+ Hits 5042 5046 +4
- Misses 942 943 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tests/xml_parser_test.c
Outdated
@@ -95,6 +95,25 @@ static int s_xml_parser_child_with_text_test(struct aws_allocator *allocator, vo | |||
|
|||
AWS_TEST_CASE(xml_parser_child_with_text, s_xml_parser_child_with_text_test) | |||
|
|||
const char *malformed = "<?xml version=\"1.0\" encoding=\"UTF-8\"?><rootNode>><child1>TestBody</child1></rootNode>"; | |||
|
|||
static int s_xml_parser_malformed_test(struct aws_allocator *allocator, void *ctx) { |
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.
nit: i would call out exactly what is malformed in this test, in case we want to add more malformed tests
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.
Thanks, updated.
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.
LGTM
source/xml_parser.c
Outdated
size_t node_name_len = end_location - next_location; | ||
|
||
size_t node_name_len; | ||
if (aws_sub_size_checked((size_t)end_location, (size_t)next_location, &node_name_len)) { |
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.
maybe use an if check against the pointer, which will be safer
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.
Thanks, updated.
Description of changes:
Our XML parser assumes that the XML provided is valid and is intended for internal use only. I have fixed buffer overflow for some cases as it's nice to fix them but there might be other issues with invalid XML.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.