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

[CVE-2022-23852] Prevent XML_GetBuffer signed integer overflow #550

Merged
merged 3 commits into from Jan 24, 2022

Conversation

hartwork
Copy link
Member

@hartwork hartwork commented Jan 23, 2022

@hartwork hartwork added this to the 2.4.4 milestone Jan 23, 2022
@hartwork hartwork force-pushed the prevent-getbuffer-overflow branch from 6c36498 to 9d94853 Compare Jan 23, 2022
expat/tests/runtests.c Outdated Show resolved Hide resolved
@rillig
Copy link

@rillig rillig commented Jan 23, 2022

Looks good to me as the code uses the same pattern as everywhere else to detect integer overflow.

@hartwork hartwork force-pushed the prevent-getbuffer-overflow branch from 9d94853 to cd1c987 Compare Jan 23, 2022
@hartwork
Copy link
Member Author

@hartwork hartwork commented Jan 23, 2022

@rillig thanks for the review 👍

CVE request submitted to Mitre just now.

@hartwork hartwork changed the title Prevent XML_GetBuffer signed integer overflow [CVE-2022-23852] Prevent XML_GetBuffer signed integer overflow Jan 24, 2022
@hartwork
Copy link
Member Author

@hartwork hartwork commented Jan 24, 2022

Got CVE-2022-23852

@hartwork hartwork force-pushed the prevent-getbuffer-overflow branch from cd1c987 to 99cec43 Compare Jan 24, 2022
@ferivoz
Copy link
Contributor

@ferivoz ferivoz commented Jan 24, 2022

Looks good to me as well. Thank you for extending the test into its own test case and adjusting the signed overflow check!

@hartwork
Copy link
Member Author

@hartwork hartwork commented Jan 24, 2022

@ferivoz thanks for the review 👍

Merging now...

@hartwork hartwork merged commit 178d26f into master Jan 24, 2022
60 checks passed
@hartwork hartwork deleted the prevent-getbuffer-overflow branch Jan 24, 2022
@iamkarlson
Copy link

@iamkarlson iamkarlson commented Jan 28, 2022

Can anyone who submitted a CVE explain in more detail what is this package for and what might be affected by using it?

@hartwork
Copy link
Member Author

@hartwork hartwork commented Jan 28, 2022

@iamkarlson are you asking what libexpat is being used for in general? Have you read the project description? For an (incomplete) list of hardware and other Open Source software using libexpat please see https://libexpat.github.io/doc/users/ .

@DominikBauer1
Copy link

@DominikBauer1 DominikBauer1 commented Jan 31, 2022

Hi, the CVSS value of 9.8 seems quite high for an integer overflow. Is there any malicious advantage that I don't see at the moment?

@hartwork
Copy link
Member Author

@hartwork hartwork commented Jan 31, 2022

@DominikBauer1 to my best knowledge it is unclear how much more than DoS can be gotten out of that overflow: The optimistic view is "probably just DoS" and the pessimistic one is "this could be code execution". That's why I wrote "Impact is denial of service or more" in the change log. I didn't suggest a score that high, but maybe the person who made that choice has information that I don't. Does that answer the question?

@DominikBauer1
Copy link

@DominikBauer1 DominikBauer1 commented Jan 31, 2022

@hartwork Yes, that answers my question. Thanks for the quick reply.

@iamkarlson
Copy link

@iamkarlson iamkarlson commented Feb 8, 2022

@hartwork yes, that's what I was looking for. We're using docker images and I wanted to remove all unpatched dependencies from them.

@hartwork
Copy link
Member Author

@hartwork hartwork commented Feb 8, 2022

@iamkarlson unless an app comes with their own bundled copy of libexpat (and most distros will patch bundles out for security reasons), updating libexpat in the distro will get the fix to all apps at once, because they all use that very instance, that's the idea. If you want to discuss this more, let's take it to e-mail, my contact is on the profile page, just so we don't spam everyone here with something that is not super specific to this very vulnerability fix. Okay? Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants