Bug 3573 - Out of bounds read in macro LBREAK_IF_REQUIRED in codeconv.c
: Out of bounds read in macro LBREAK_IF_REQUIRED in codeconv.c
Status: RESOLVED FIXED
Product: Claws Mail
Classification: Unclassified
Component: Other
: 3.13.1
: PC Linux
: P3 normal
Assigned To: users
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-11-29 11:53 CET by Hanno Boeck
Modified: 2016-08-03 10:36 CEST (History)
0 users

See Also:


Attachments
[patch] Fix invalid memory access in LBREAK_IF_REQUIRED (451 bytes, patch)
2015-11-29 11:53 CET, Hanno Boeck
no flags Details | Diff
full address sanitizer stack trace (5.50 KB, text/plain)
2015-11-29 11:54 CET, Hanno Boeck
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Hanno Boeck 2015-11-29 11:53:01 CET
Created attachment 1613 [details]
[patch] Fix invalid memory access in LBREAK_IF_REQUIRED

I discovered an out of bounds read in claws-mail when trying to reply to certain mails when testing with Address Sanitizer.

I figured out the code causing this is in the macro LBREAK_IF_REQUIRED in codeconv.c. This is the code in question:


		} else if (destp == (guchar *)dest && left < 7) {	\
			if (isspace(*(destp - 1)))			\
				destp--;				\
			else if (is_plain_text && isspace(*srcp))	\
				srcp++;					\

If I understand the code correctly the (isspace(*(destp - 1))) does not make any sense. It only gets triggered if destp and dest are identical, thus it means destp points to the beginning of the buffer. Therefore destp-1 is always pointing to invalid memory.

(This check probably got copied from some lines above. There the check is valid, because that code part gets executed when destp is bigger than dest).

So this part of the if-clause should be removed. Patch attached.
Comment 1 Hanno Boeck 2015-11-29 11:54:07 CET
Created attachment 1614 [details]
full address sanitizer stack trace
Comment 2 users 2016-07-14 12:00:03 CEST
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-07-14 12:00:03.138710625 +0200
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=a04ad79d847a1b30f9f8b397b8f250b9b846c247
Merge: 55c93fe 74b625c
Author: Colin Leroy <colin@colino.net>
Date:   Thu Jul 14 12:00:02 2016 +0200

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

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=74b625c53e38ed53a585fe43df23f12d50d447a6
Author: Ricardo Mones <ricardo@mones.org>
Date:   Thu Jul 14 11:59:22 2016 +0200

    Update lists of authors

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=4bee63af358db61367f987ef1b3dbad5070e1cad
Author: Ricardo Mones <ricardo@mones.org>
Date:   Thu Jul 14 11:52:43 2016 +0200

    Fix bug #3573: Out of bounds read in macro LBREAK_IF_REQUIRED
    
    Thanks Hanno Boeck for the patch!