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
MDEV-31566 Fix buffer overrun of column_json function #3242
Conversation
Hi @tohghua, please follow the PR template for contributions
Furthermore, your commit message is very vague and does not describe what the issue you are fixing is, why you are fixing it or when it occurs. Please consider adding some more detail. Furthermore, you describe in the current PR description a test case that should pass with the addition of your new changes. Perhaps add an MTR test to verify this as well. Here is some good documentation you can refer to for what I've mentioned above: |
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.
Implementation looks good.
Thanks for signing the CLA.
The commit message will need to be improved and test case added for this to merge (back to 10.5 branch).
If you want use to take care of these details please let us know.
@@ -3865,17 +3865,18 @@ my_bool dynstr_append_json_quoted(DYNAMIC_STRING *str, | |||
} | |||
else | |||
{ | |||
if (lim < 2) |
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.
so we moved this because there could be up to 2 more characters. We could be over allocating, but I'm not sure it matters.
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.
Over allocating should be fine, it's just a buffer.
If you want us to take care of these details please let us know.
You're welcome :)
lim--; | ||
str->str[str->length++]= '\\'; | ||
} | ||
lim--; |
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.
good.
The accounting of the limit variable that represents the amount of space left it the buffer was incorrect. Also there was 1 or 2 bytes left to write that occured without the buffer length being checked. Review: Sanja Byelkin
thank you for the fix @tohghua. Sorry I was slow applying your first fix. |
See https://jira.mariadb.org/browse/MDEV-31566
Test case: the following sql should work fine.
select column_json(0x0402000A0000000300030023076A736E7375626A6563742E0005006C0027000200290002002B0002002D0002002F0002000C31000C3B000C4B000C51000F62006631663266336634663509E5A79AE8BF9CE6B48B0FE8819AE9809AE98791E6A1A5E5BA970537343530301031313634332F393634352F31313630300C080000000000EFBFBDEFBFBD192E);
select column_json(0x0402000900000003000300740C6A736E766F6C756D652E000900EFBFBD004300020045000200470003004A0004004E00050053000500580005005D000500620005000C67000C6A000C6D000C7000052C00051B00052C000CEFBFBD0007EFBFBD006638663966313070696332626F785F63626F785F67626F785F6B626F785F7666355F696402343402343402333241687474703A2F2F6F73732E68646238382E636F6D2F302F70686F746F2F30373865653765376336343634616236386130343833373333323636613532612E67696608302E303532323732244F1E00030180C106);