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.

Sign up for GitHub

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

Jump to bottom

CVEs - Issue #2119 #2133

Closed
wants to merge 7 commits into from
Closed

CVEs - Issue #2119 #2133

wants to merge 7 commits into from

Conversation

e4t
Copy link
Contributor

@e4t e4t commented Oct 29, 2021

This series of patches addresses the CVEs mentioned in issue #2119 that have not been addressed by commit b5d4afd.

e4t added 7 commits October 29, 2021 10:25
@e4t
Fix for CVE-2019-20200 / ezxml bug 19
7a8e1f8 
Make sure to not read past end of string after deleting '\r'.
This fixes
   https://sourceforge.net/p/ezxml/bugs/19/

Signed-off-by: Egbert Eich <eich@suse.com>
@e4t
Fix for CVE-2019-20006/CVE-2019-20202/CVE-2021-31598 ezxml bug 15/17/28
1ff1a1a 
For UTF-8 the multi-byte sequences should use at most 36 bits
UTF-8 standard uses 21 bits or 4 bytes).
This fixes:
   https://sourceforge.net/p/ezxml/bugs/15/
   https://sourceforge.net/p/ezxml/bugs/17/
   https://sourceforge.net/p/ezxml/bugs/28/

Signed-off-by: Egbert Eich <eich@suse.com>
@e4t
Fix for CVE-2019-20199 / ezxml bug 18
f17f1d3 
Make sure end token ';' has really been found.
This fixes
  https://sourceforge.net/p/ezxml/bugs/18/

Signed-off-by: Egbert Eich <eich@suse.com>
@e4t
Fix for CVE-2019-20007 / ezxml bug 13
1078a13 
Make sure that ezxml_str2utf8() has succeeded.
This fixes
  https://sourceforge.net/p/ezxml/bugs/13/

Signed-off-by: Egbert Eich <eich@suse.com>
@e4t
Fix for CVE-2021-26221 / ezxml bug 21
1df77da 
Make sure malloc() succeeds.

This fixes
 https://sourceforge.net/p/ezxml/bugs/21/

Signed-off-by: Egbert Eich <eich@suse.com>
@e4t
Fix for CVE-2021-26222 / ezxml bug 22
7bd0ba3 
Make sure malloc() succeeds.
This fixes
 https://sourceforge.net/p/ezxml/bugs/22/

Signed-off-by: Egbert Eich <eich@suse.com>
@e4t
Back out bogus fix for CVE-2021-31598
3657dc9 
The original fix for CVE-2021-31598 from
 https://sourceforge.net/p/ezxml/bugs/28/
is bogus:
1. The memmove target (s + c) may not point to a valid string
   thus the result of the test is invalid and may in fact break
   on perfectly valid XML data.
2. Using exit(-1) when the test fails to hard-exit the program
   is prohibitiv for a library like netcdf.
The issue reported in above ticket is resolved by the fix for
CVE-2019-20006: https://sourceforge.net/p/ezxml/bugs/15/
and
CVE-2019-20202: https://sourceforge.net/p/ezxml/bugs/17/

Signed-off-by: Egbert Eich <eich@suse.com>
@e4t e4t requested a review from WardF as a code owner October 29, 2021 10:40
@e4t e4t mentioned this pull request Oct 29, 2021
Closed
@e4t
Copy link
Contributor Author

e4t commented Oct 29, 2021

A more thorough analysis if the issues discovered can be found here.

@DennisHeimbigner
Copy link
Collaborator

I am a bit confused. In your list, the only open bug appears to be bullet point 1, correct?

@DennisHeimbigner
Copy link
Collaborator

Sounds like the correct solution is to update to the latest version of exzml, correct?

@e4t
Copy link
Contributor Author

e4t commented Oct 30, 2021

I am a bit confused. In your list, the only open bug appears to be bullet point 1, correct?

Actually, no. The only issues fixed in netcdf-c from the list in #2119 so far are:

  • CVE-2019-20198, CVE-2019-20201 CVE-2021-31229 (same root cause)
  • CVE-2021-31229
  • CVE-2021-30485
    Of the remaining ones, the patch set in this pull request fixes all published reproducers - except for CVE-2019-20005: here the published reproducer does not triggers any issue for me.
    However: CVE-2021-2622, CVE-2021-26222 and CVE-2021-26220 as well as CVE-2019-20198, CVE-2019-20201 and CVE-2021-31229 are only instances of a class of problems (not checking the return values of malloc(), not checking where the return value of str[c]spn() points to) - these two classes of problems are not addressed at all, yet:
  • ezxml.c calls malloc()/realloc() 41times. Only 3 of these are addressed with the patch set above.
  • ezxml.c calls strcspn() 10 times, strspn() 15 times, only instance str has been fixed so far.

Sounds like the correct solution is to update to the latest version of exzml, correct?

As far as I can see, the latest exzml is used already. As it looks like, the upstream project has been abandoned for a while already. I've published my proposed fixes as attachment to the respective bug and do not intend to take over maintenance of ezxlm.
There are a number of ezxml clones/forks here on github, but all of these appear to be in an equally abandoned state.
The correct solution would be to switch to a better curated XML parser that is actually maintained and cared for by upstream.
Pros and cons of using to libxml are discussed in #2119.

@DennisHeimbigner
Copy link
Collaborator

I think all of these changes are covered by PR #2139

@e4t
Copy link
Contributor Author

e4t commented Nov 8, 2021

I think all of these changes are covered by PR #2139

Yes, I've seen this. Thank you!
I still would've preferred if these were merged to keep the change logs intact which reference the CVEs.

@e4t e4t closed this Nov 8, 2021
@Dave-Allured Dave-Allured mentioned this pull request Dec 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WardF WardF Awaiting requested review from WardF WardF is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@e4t @DennisHeimbigner