Bug 3557 - Remotely exploitable bug.
Summary: Remotely exploitable bug.
Status: RESOLVED FIXED
Alias: None
Product: Claws Mail
Classification: Unclassified
Component: Other (show other bugs)
Version: 3.13.1
Hardware: PC Linux
: P3 critical
Assignee: users
URL:
Depends on:
Blocks:
 
Reported: 2015-11-04 22:12 UTC by DrWhax
Modified: 2016-01-17 12:34 UTC (History)
0 users

See Also:


Attachments
Correct and comment range checks in Japanese text conversions (1.56 KB, patch)
2016-01-12 22:49 UTC, Ben Hutchings
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description DrWhax 2015-11-04 22:12:19 UTC
Hi,

I'm a Tails(https://tails.boum.org/) contributor, yesterday, we came across anonymous posts on an image board where people were dropping bugs on Tails or software we ship. Apparantly, Claws might be affected.

So in codeconv.c there is a function for japanese character set conversion called conv_jistoeuc().  There is no bounds checking on the output buffer, which is created on the stack with alloca().  

Bug can be triggered by sending an email to TAILS_luser@riseup.net or whatever.

Since my C is completely rusty, you might be able to make a better judgement on the severity of this issue. Marking critical for now.
Comment 1 users 2015-11-04 22:41:02 UTC
Changes related to this bug have been committed.
Please check latest Git and update the bug accordingly.
You can also get the patch from:
http://git.claws-mail.org/

++ ChangeLog	2015-11-04 22:41:02.210154596 +0100
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=05163d95b06cebae2b5c4c48b99c3feece761604
Merge: 23e4794 d390fa0
Author: Colin Leroy <colin@colino.net>
Date:   Wed Nov 4 22:41:01 2015 +0100

    Merge branch 'master' of file:///home/git/claws

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=d390fa07f5548f3173dd9cc13b233db5ce934c82
Author: Colin Leroy <colin@colino.net>
Date:   Wed Nov 4 22:40:32 2015 +0100

    Make sure we don't run out of the output buffer. Maybe fixes bug #3557
Comment 2 Colin Leroy 2015-11-04 22:42:16 UTC
Thanks for the heads up. I could see how it would be possible to run out of the output buffer in this function (and two others). I would gladly have more information though. The email address you provided bounces.
Comment 3 DrWhax 2015-11-04 22:48:20 UTC
(In reply to comment #2)
> Thanks for the heads up. I could see how it would be possible to run out of
> the output buffer in this function (and two others). I would gladly have
> more information though. The email address you provided bounces.

<no idea why this e-mail address bounces, I get e-mail through bugzilla, i'll whitelist your e-mail adress.>

Thanks for the quick fix! 

Unfortunately, that's the only information that was published on this, I don't think I can be of more help. 

I'm letting a few more people look at the patch so we're sure it's good enough.

Cheers!
Comment 4 Colin Leroy 2015-11-04 23:06:06 UTC
Hi,

I meant this address : TAILS_luser@riseup.net

Thanks for eyeballing the patch anyway.
Comment 5 DrWhax 2015-11-04 23:07:07 UTC
(In reply to comment #4)
> Hi,
> 
> I meant this address : TAILS_luser@riseup.net
> 
> Thanks for eyeballing the patch anyway.

Ah, that's just an example/random user :)
Comment 6 Ricardo Mones 2015-12-30 14:50:10 UTC
After being notified of this:

https://security-tracker.debian.org/tracker/CVE-2015-8614

Seems this is only partially fixed (wrong operator was fixed in #3584), and there's code paths which exceed the number of reserved chars for output.

Similar functions in libsylph¹ are unaffected², so those could be used instead.

¹ http://sylpheed.sraoss.jp/redmine/projects/sylpheed/repository/entry/libsylph/codeconv.c 
² https://lists.debian.org/debian-lts/2015/12/msg00104.html
Comment 7 Ben Hutchings 2015-12-31 04:22:35 UTC
(In reply to comment #6)
> After being notified of this:
> 
> https://security-tracker.debian.org/tracker/CVE-2015-8614
> 
> Seems this is only partially fixed (wrong operator was fixed in #3584), and
> there's code paths which exceed the number of reserved chars for output.

Right.  In conv_euctojis() the comparison is with outlen - 3, but each pass through the loop uses up to 5 bytes and the rest of the function may add another 4 bytes.  The comparison should presumably be '<= outlen - 9' or equivalently '< outlen - 8'.

> Similar functions in libsylph¹ are unaffected², so those could be used
> instead.

The corresponding functions in libsylph do their own allocations on the heap, returning a pointer to the caller.  So it's not quite as simple as copying the code across.
Comment 8 Ricardo Mones 2015-12-31 12:03:40 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > After being notified of this:
> > 
> > https://security-tracker.debian.org/tracker/CVE-2015-8614
> > 
> > Seems this is only partially fixed (wrong operator was fixed in #3584), and
> > there's code paths which exceed the number of reserved chars for output.
> 
> Right.  In conv_euctojis() the comparison is with outlen - 3, but each pass
> through the loop uses up to 5 bytes and the rest of the function may add
> another 4 bytes.  The comparison should presumably be '<= outlen - 9' or
> equivalently '< outlen - 8'.

Thanks for confirming Ben.

> > Similar functions in libsylph¹ are unaffected², so those could be used
> > instead.
> 
> The corresponding functions in libsylph do their own allocations on the
> heap, returning a pointer to the caller.  So it's not quite as simple as
> copying the code across.

Indeed. I was thinking about a more radical approach like removing codeconv.[ch] and use sylph/codeconv.h, linking to libsyph. But I had a look today and this is also a huge task:

• the API calls interface changes (obviously)
• different supported codesets on both sides (libsylph should add some so CM doesn't lose features)
• missing calls

Maybe could be a good idea for long term, but not for tonight ;-)
Comment 9 Ben Hutchings 2016-01-12 22:49:00 UTC
Created attachment 1623 [details]
Correct and comment range checks in Japanese text conversions
Comment 10 users 2016-01-17 12:35:03 UTC
Changes related to this bug have been committed.
Please check latest Git and update the bug accordingly.
You can also get the patch from:
http://git.claws-mail.org/

++ ChangeLog	2016-01-17 12:35:02.882777161 +0100
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=6f24db001cd21dfef5466e54a6c0d2f1f9cbcd63
Merge: 01b39bc 8b2aff8
Author: Colin Leroy <colin@colino.net>
Date:   Sun Jan 17 12:35:02 2016 +0100

    Merge branch 'master' of file:///home/git/claws

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=8b2aff884d97dcfe5cc70478fecc7c87ce023c95
Author: Paul <paul@claws-mail.org>
Date:   Sun Jan 17 11:34:14 2016 +0000

    fix CVE-2015-8708, bug 3557, 'Remotely exploitable bug.'
    
    Patch by Ben Hutchings <ben@decadent.org.uk>