libytnef: February 2017 multiple vulnerabilities (X41-2017-002)

Bug #1666884 reported by Jeremy Bícha
270
This bug affects 3 people
Affects Status Importance Assigned to Milestone
libytnef (Debian)
Fix Released
Unknown
libytnef (Ubuntu)
Fix Released
Medium
Unassigned
Trusty
Confirmed
Medium
Unassigned
Xenial
Incomplete
Medium
Jeremy Bícha
Yakkety
Won't Fix
Medium
Jeremy Bícha
Zesty
Won't Fix
Medium
Jeremy Bícha

Bug Description

http://www.openwall.com/lists/oss-security/2017/02/15/4

https://github.com/Yeraze/ytnef/pull/27/files

Upstream calls this X41-2017-002 but a bunch of CVEs have been assigned too.
https://security-tracker.debian.org/tracker/source-package/libytnef

Fixed in zesty. I'd like to copy the Debian stable security patches when it's released there.

Quoting from the oss-security post…

Summary and Impact
------------------
Multiple Heap Overflows, out of bound writes and reads, NULL pointer
dereferences and infinite loops have been discovered in ytnef 1.9 an
earlier.
These could be exploited by tricking a user into opening a malicious
winmail.dat file.

Product Description
-------------------
ytnef offers a library and utilities to extract the files from winmail.dat
files. winmail.dat files are send by Microsoft Outlook when forwarding files
via e-mail. The vendor was very responsive in providing a patched version.

Analysis
--------
Due to the big amount of issues found no detailed analysis is given here.
Almost all allocations were unchecked and out of bounds checks rarely
performed in the code.

In total 9 patches were generated for the following issues:

1. Null Pointer Deref / calloc return value not checked
2. Infinite Loop / DoS
3. Buffer Overflow in version field
4. Out of Bound Reads
5. Integer Overflow
6. Invalid Write and Integer Overflow
7. Out of Bounds read
8. Out of Bounds read and write
9. Directory Traversal using the filename

Testing Done
------------
None

Other Info
----------
Zesty already got these fixes synced from Debian. Trusty got these fixes earlier in May since it was still in main. Recently, there's one more CVE, 2017-9058 so I've supplied debdiffs for trusty and zesty for that issue, copied from Debian's 1.9.2-2 package (which will autosync to artful). For xenial and yakkety, I also added the patches that were applied to trusty.

For more about this new issue, see Debian bug 862556

The only reverse dependency for libytnef is evolution.

For xenial and yakkety, the CVE patch appears to have a basically duplicate fix for the second half of pt_clsid.diff so I dropped those lines from pt_clsid.diff.

Changed in libytnef (Ubuntu):
status: New → Confirmed
Jeremy Bícha (jbicha)
description: updated
Revision history for this message
Jeremy Bícha (jbicha) wrote :
tags: added: patch trusty xenial yakkety zesty
Revision history for this message
Jeremy Bícha (jbicha) wrote :
Revision history for this message
Jeremy Bícha (jbicha) wrote :
Revision history for this message
Jeremy Bícha (jbicha) wrote :
description: updated
Jeremy Bícha (jbicha)
Changed in libytnef (Ubuntu Trusty):
status: New → Confirmed
Changed in libytnef (Ubuntu Xenial):
status: New → Confirmed
Changed in libytnef (Ubuntu Yakkety):
status: New → Confirmed
Changed in libytnef (Ubuntu Zesty):
status: New → Confirmed
description: updated
description: updated
Jeremy Bícha (jbicha)
description: updated
Changed in libytnef (Debian):
status: Unknown → Fix Released
Revision history for this message
Tyler Hicks (tyhicks) wrote :

Thanks for the debdiffs! The only change that I made was to the version used in the Zesty debdiff. I changed 1.9.2-1ubuntu0.17.04 to 1.9.2-1ubuntu0.1 as suggested here:

  https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation#Update_the_packaging

I've uploaded the packages to ppa:ubuntu-security-proposed/ppa

Once the builds complete, please perform testing of xenial, yakkety, and zesty and then update this bug report with the testing that you performed.

I'll need to perform my own testing of libytnef on trusty since it is in main for that release. Feel free to skip it.

Changed in libytnef (Ubuntu Trusty):
assignee: nobody → Tyler Hicks (tyhicks)
Changed in libytnef (Ubuntu Xenial):
assignee: nobody → Jeremy Bicha (jbicha)
Changed in libytnef (Ubuntu Yakkety):
assignee: nobody → Jeremy Bicha (jbicha)
Changed in libytnef (Ubuntu Zesty):
assignee: nobody → Jeremy Bicha (jbicha)
status: Confirmed → Incomplete
Changed in libytnef (Ubuntu Yakkety):
status: Confirmed → Incomplete
Changed in libytnef (Ubuntu Xenial):
status: Confirmed → Incomplete
Revision history for this message
Tyler Hicks (tyhicks) wrote :

The testing for the Trusty update did not go as expected. The test case linked to from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=862556#5 crashes Evolution the same way with and without the updated libytnef0 package.

Testing on Trusty isn't straightforward because Evolution's handling of tnef attachments is buggy in Trusty. You have to use this workaround:

  https://bugs.launchpad.net/ubuntu-gnome/+bug/1390466/comments/2

I then used mutt to create an email with the previously mentioned reproducer tnef file attached. Before sending, I had to manually set the attachment content type to "application/ms-tnef". (In hindsight, I could have probably set the content type to "application/ms-tnefl" and then not needed to binary patch the Evolution tnef module.)

Evolution crashes as soon as you click on the email containing the crafted tnef file. The backtrace is:

(gdb) bt
#0 0x00007fb1ec652be8 in TNEFFreeMapiProps () from /usr/lib/x86_64-linux-gnu/libytnef.so.0
#1 0x00007fb1ec652e1b in TNEFFree () from /usr/lib/x86_64-linux-gnu/libytnef.so.0
#2 0x00007fb1ec869dcd in ?? () from /usr/lib/evolution/3.10/modules/module-tnef-attachment.so
#3 0x00007fb1f737060c in e_mail_parser_parse_part_as () from /usr/lib/evolution/3.10/libevolution-mail-formatter.so.0
#4 0x00007fb1f73706cd in e_mail_parser_parse_part () from /usr/lib/evolution/3.10/libevolution-mail-formatter.so.0
#5 0x00007fb1f73731b2 in ?? () from /usr/lib/evolution/3.10/libevolution-mail-formatter.so.0
#6 0x00007fb1f737060c in e_mail_parser_parse_part_as () from /usr/lib/evolution/3.10/libevolution-mail-formatter.so.0
#7 0x00007fb1f73718cb in ?? () from /usr/lib/evolution/3.10/libevolution-mail-formatter.so.0
#8 0x00007fb1f736ff12 in ?? () from /usr/lib/evolution/3.10/libevolution-mail-formatter.so.0
#9 0x00007fb1f73700b1 in e_mail_parser_parse_sync () from /usr/lib/evolution/3.10/libevolution-mail-formatter.so.0
#10 0x00007fb1eeac09bd in ?? () from /usr/lib/evolution/3.10/libevolution-mail.so.0
#11 0x00007fb21dc4f2af in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#12 0x00007fb21dc3c4e6 in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#13 0x00007fb21dc5f065 in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#14 0x00007fb21d6f388c in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#15 0x00007fb21d6f2f05 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#16 0x00007fb220cb0184 in start_thread (arg=0x7fb1dbfff700) at pthread_create.c:312
#17 0x00007fb21d3babed in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111

I don't plan to move forward with this update since it doesn't fix the crasher.

Changed in libytnef (Ubuntu Trusty):
assignee: Tyler Hicks (tyhicks) → nobody
Revision history for this message
Tyler Hicks (tyhicks) wrote :

Unsubscribing ubuntu-security-sponsors. Please subscribe the team if new debdiffs are available.

Revision history for this message
Michael Gratton (mjog) wrote :

The remaining CVE's have recently been fixed (or will be once the last MR lands) in the library's repo. Also, importantly, the one CVE fix that Ubuntu did ship last year broke the library's normal operation, making it less than useful for decoding

Resubscribing ubuntu-security-sponsors since while these aren't debdiffs, it would be good to get the package updated to address the remaining CVEs and restore functionality.

Revision history for this message
Seth Arnold (seth-arnold) wrote :

Hello Michael, do you have a bug report for the regression?

Thanks

Revision history for this message
Michael Gratton (mjog) wrote :

Hey Seth, have a look at the last two comments in the original ticket for the first CVE that was reported: https://github.com/Yeraze/ytnef/issues/45#issuecomment-393044169 . The PR with the proper fix for the CVE mentioned there (https://github.com/Yeraze/ytnef/pull/58) has already been merged by the maintainer. Note it depends on at least one other PR as well.

The person that developed that ytnef PR did so after looking at adding TNEF support to Geary and noticing that ytnef was reasonably broken on some distros, including Ubuntu. Looking into it, it seems those where it is broken shipped the patch from the original CVE.

Recently some additional issues have been reported, there's a yet-to-be-merged PR for those as well: https://github.com/Yeraze/ytnef/pull/71

Revision history for this message
Leonidas S. Barbosa (leosilvab) wrote :

Hi Michael,

What is the version that is causing regression? Right now we are in 1.5-6ubuntu0.2 that address some CVEs and issue 58 (CVE-2017-9058) but trusty hasn't the ytnefprint.

Would you mind to point us some ways to reproduce this?

Thanks!

Revision history for this message
Michael Gratton (mjog) wrote :

Hey Leonidas,

Per this comment: https://github.com/Yeraze/ytnef/issues/45#issuecomment-392658096, if you download this example file https://bugs.mageia.org/attachment.cgi?id=9088, then run a version of ytnef with the patch from CVE-2017-9058 applied to it (e.g. libytnef0 1.9.2-2), you'll see the following:

> mjg@payens:~$ ytnef -v -f . ~/winmail.dat
> Corrupted file detected at ytnef.c : 546
> zappa_av1.jpg

In particular it doesn't successfully extract the file bookmark.htm from the example.

What you should see instead is (this is running current yntef git master):

> mjg@payens:~/local/src/ytnef$ ytnef/ytnef -v -f . ~/winmail.dat
> Attempting to parse /home/mjg/Incoming/winmail.dat...
> ./zappa_av1.jpg
> ./bookmark.htm

Does that help?

Revision history for this message
Leonidas S. Barbosa (leosilvab) wrote :

Hey Michael,

For trusty, that is the only version we have in main, and the one I did a sec update with CVE-2017-9058 it doesn't support ytnef tool, only the libytnef0 and current version is 1.5-6ubuntu0.2. It maybe indicates that trusty was not affected. I'll spend sometime on this later and verify if it's there any way to check/test that POC for trusty.

Thanks for point us the ways to check/test it!

Revision history for this message
Michael Gratton (mjog) wrote :

Hey Leonidas,

Thanks for looking into this. It would be good to see if the updated fixes can be applied to all currently supported releases, especially since people are more likely to be running Xenial or Bionic, as well as Cosmic, so we can rely on it working going forward.

Cheers!

Revision history for this message
Oliver Giles (ohw-giles) wrote :

Hi, I implemented those fixes to libytnef. Yeraze has just released 1.9.3 so I'm interested to see if/when it will make it to Ubuntu, and to which releases.

The ytnef and ytnefprint binaries just call libytnef, both the wrong and the right fixes to CVE-2017-9068 are definitely part of the library, so I would say that trusty is most likely affected. There are also several other equally severe CVEs which have been reported and fixed since version 1.5.

Revision history for this message
Leonidas S. Barbosa (leosilvab) wrote :

Hi Oliver,

Thanks for the comments...

For trusty I did an update applying:

From 0eab0e46f4828839a7f7e46e48fc33167377ec0d Mon Sep 17 00:00:00 2001
From: Oliver Giles <email address hidden>
Date: Wed, 30 May 2018 09:06:02 +0300
Subject: [PATCH] Fix length-check before populating propnames

The earlier length check did not check enough bytes. But rather
than fixing the off-by-one, it makes more sense to do a single
check at the start of the loop.

Resolves CVE-2017-9058.

Although, the second piece of the code/patch wasn't applied to trusty because it hasn't ytnefprint. I'm not sure if I got it right, but you are meaning even for Trusty only this patch doesn't solve the issue?

@Michael, I agree with you, but right now for bionic and xenial this package is in universe what means it's a community question of time it be update with those CVEs.

tags: added: community-security
Revision history for this message
Oliver Giles (ohw-giles) wrote :

Apologies for the late reply, I neglected to enable notifications...

No, I just meant that the unpatched Trusty package isn't safe just because it doesn't contain ytnef/ytnefprint binaries. You have it right, the single patch you mention will be enough to address CVE-2017-9058. It should replace this patch[1]. That will at least restore correct behaviour to the library.

However, this bugreport mentions several vulnerabilites, and the patch only covers CVE-2017-9058. As you can see on the github releases page[2], there have been many CVEs addressed in the past few releases. I don't know how feasible this is but if possible I highly recommend upgrading to 1.9.3.

[1] https://sources.debian.org/patches/libytnef/1.9.2-2/CVE-2017-9058.patch/
[2] https://github.com/Yeraze/ytnef/releases

Steve Beattie (sbeattie)
Changed in libytnef (Ubuntu Yakkety):
status: Incomplete → Won't Fix
Changed in libytnef (Ubuntu Zesty):
status: Incomplete → Won't Fix
Revision history for this message
Oliver Giles (ohw-giles) wrote :

Pretty sure this also affects bionic and cosmic

Mathew Hodson (mhodson)
Changed in libytnef (Ubuntu):
importance: Undecided → Medium
Changed in libytnef (Ubuntu Trusty):
importance: Undecided → Medium
Changed in libytnef (Ubuntu Xenial):
importance: Undecided → Medium
Changed in libytnef (Ubuntu Yakkety):
importance: Undecided → Medium
Changed in libytnef (Ubuntu Zesty):
importance: Undecided → Medium
Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

Someone needs to attach updated debdiffs to fix the CVEs, including the regression fix and the latest round of CVE fixes.

Unsubscribing ubuntu-security-sponsors for now. Please re-subscribe the team once new debdiffs have been uploaded.

Thanks!

Revision history for this message
Michael Gratton (mjog) wrote :

Attached patch updates from from 1.9.2 to 1.9.3

Revision history for this message
Michael Gratton (mjog) wrote :

Re-subscribed ubuntu-security-sponsors - the attached patch fixes the CVEs.

NB despite the gz filename, it's actually a plain text patch. Apologies.

Revision history for this message
Michael Gratton (mjog) wrote :

Marc, anything else needed to be done here?

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

I can't actually see the patch in comment #20, I'm getting an encoding error. Are you seeing something similar?

Revision history for this message
Michael Gratton (mjog) wrote :

Ah, my bad. Here it is again.

It's actually just same as for the new version in cosmic. :)

Revision history for this message
Seth Arnold (seth-arnold) wrote :

I've got some concerns about this:

$ diffstat !$
diffstat libytnef_1.9.2-2_1.9.3-1.diff
 ChangeLog | 16 +++
 configure.ac | 2
 debian/changelog | 24 +++++
 debian/compat | 2
 debian/control | 13 +-
 debian/patches/CVE-2017-9058.patch | 13 --
 debian/patches/series | 1
 lib/ytnef.c | 170 ++++++++++++++++++++-----------------
 ytnef/main.c | 37 ++++----
 ytnefprint/main.c | 2
 10 files changed, 168 insertions(+), 112 deletions(-)

The package is managed with quilt patches but there are significant changes to five files made directly to the files rather than via quilt patches. (Of those, only the changes to the .c files look like security fixes, but those should be handled via individual patches, similar to the now-removed CVE-2017-9058.patch.) The changelog still mentions Debian unstable rather than a specific Ubuntu release.

How did you test your changes?

Thanks

Revision history for this message
Michael Gratton (mjog) wrote :

Seth, no I didn't, it's just the debdiff taken from the dingo source package: https://launchpad.net/ubuntu/+source/libytnef/1.9.3-1

I admit this is pretty lazy, but assumed that since it was fine for dingo it would be fine here.

Mathew Hodson (mhodson)
Changed in libytnef (Ubuntu):
status: Confirmed → Fix Released
Revision history for this message
Simon Quigley (tsimonq2) wrote :

Unsubscribing the Security Sponsors team, please resubscribe when Seth's comments have been addressed.

Thanks!

To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.