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

[0.26] try backporting fixes for CVE-2018-{12264,12265} #398

Merged
merged 8 commits into from
Aug 11, 2018

Conversation

vcunat
Copy link

@vcunat vcunat commented Aug 4, 2018

I tried backporting the missing CVE commits from #365 and #366, but someone actually knowing this project's code should have a look. Perhaps @D4N?

  • Add offset_ and size_... had only a trivial conflict around the includes, so I haven't really read it properly.
  • Fix addition overflows... wasn't so trivial, as the code in the second hunk has changed a bit.

D4N added 2 commits August 4, 2018 19:18
Several checks for extracted values performed no overflow checks on the
addition. They can be tricked into passing, albeit the individual summands are
too large.
=> use Safe::add() which now aborts when an overflow occurs
This fixes Exiv2#366

(cherry picked from commit fe70939)
offset_ can become arbitrarily large and overflows once its added to size_,
this causes all kinds of problems further in the code when offset_ is used
again.
=> Use Safe::add() to catch potential overflows
This fixes Exiv2#365.

(cherry picked from commit 937a1a2)
@D4N
Copy link
Member

D4N commented Aug 5, 2018

Please also try to cherry-pick the following commits:

They add the reproducers to the test suite, surround the exiv2-cli program with a try {..} catch {..} block and fix the test names.

D4N added 4 commits August 5, 2018 17:16
It effectively looks the same as before, only now we don't call abort() but
instead clean up everything gracefully.

(cherry picked from commit f4e8ed2)
Issues got a CVE assigned

(cherry picked from commit e67910a)
@vcunat
Copy link
Author

vcunat commented Aug 5, 2018

OK, added. Only trivial conflicts in there.

I wasn't able to run make tests; it seems I'm missing something about how they should be run...

@vcunat vcunat force-pushed the 0.26-cve-roundup-46 branch from 10f6128 to 65f1fcb Compare August 5, 2018 15:46
D4N and others added 2 commits August 5, 2018 17:53
The testsuite now uses python's template module for string substitutions which
allows for a more natural substitution syntax known from the shell. Also, it
allows to run the substitutions multiple times, which is not possible with
string.format().
The heavy-lifting is now performed via a metaclass, which expands all variables
on the class creation.

(cherry picked from commit bd9d085)
@vcunat
Copy link
Author

vcunat commented Aug 5, 2018

OK, by misusing travis I fixed the tests somehow, though I imagine you may very likely prefer a different way :-)

Copy link
Member

@D4N D4N left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work!

@D4N D4N merged commit e7ffd83 into Exiv2:0.26 Aug 11, 2018
@vcunat vcunat deleted the 0.26-cve-roundup-46 branch August 11, 2018 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants