| Summary: | Out of bounds read in macro LBREAK_IF_REQUIRED in codeconv.c | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Claws Mail (GTK 2) | Reporter: | Hanno Boeck <hanno> | ||||||
| Component: | Other | Assignee: | users | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | normal | ||||||||
| Priority: | P3 | ||||||||
| Version: | 3.13.1 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Linux | ||||||||
| Attachments: |
|
||||||||
Created attachment 1614 [details]
full address sanitizer stack trace
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! |
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.