Bug 3559

Summary: Opening preferences window causes out of bounds read
Product: Claws Mail Reporter: Hanno Boeck <hanno>
Component: UIAssignee: users
Status: RESOLVED FIXED    
Severity: normal    
Priority: P3    
Version: other   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch to fix out of bounds
none
Address sanitizer error message none

Description Hanno Boeck 2015-11-07 14:51:12 CET
Created attachment 1594 [details]
Patch to fix out of bounds

I was testing claws-mail compiled with the compiler feature address sanitizer (-fsanitize=address in CFLAGS). This uncovered an out of bounds read when trying to open the preferences window.

The code in question is here in the file src/gtk/prefswindow.c (line 323):
			if (find_name.found && page->path[i] != page->path[i-1]) {

The problem is that this is in a loop over i starting at 0. Therefore in the first iteration it will try to access page->path[-1] and that is invalid.

The attached patch should fix it. Also attaching the full address sanitizer stack trace.
Comment 1 Hanno Boeck 2015-11-07 14:52:09 CET
Created attachment 1595 [details]
Address sanitizer error message
Comment 2 Andrej Kacian 2015-11-07 15:17:10 CET
Comment on attachment 1594 [details]
Patch to fix out of bounds

The whole equality check seems weird to me - if two path components of a PrefsPage would point to the same address in memory, something is wrong. Maybe the original author of this part of the code meant to use strcmp() instead?
Comment 3 users 2015-11-15 14:46:02 CET
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-15 14:46:02.517018012 +0100
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=e8971e437a9b10c3ab60255d198d02bfaa61fe88
Merge: 71a24cf 0314464
Author: Colin Leroy <colin@colino.net>
Date:   Sun Nov 15 14:46:02 2015 +0100

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

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=0314464dd5c5e4e49f7fb5835e0316726267506e
Author: Paul <paul@claws-mail.org>
Date:   Sun Nov 15 13:45:43 2015 +0000

    fix bug 3559, 'Opening preferences window causes out of bounds read'
    
    patch by Hanno Boeck
Comment 4 users 2015-11-17 11:02:02 CET
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-17 11:02:02.455764696 +0100
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=63af23e061abfdb6aacb26791cd9d7450788dc17
Merge: c7030e7 4c14c38
Date:   Tue Nov 17 11:02:01 2015 +0100
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=4c14c38f44d17da42f7817ecf04b3c1d6d6ccf5c
Author: Ricardo Mones <ricardo@mones.org>
Date:   Tue Nov 17 10:59:51 2015 +0100

    Fix bug #3565 by reverting “fix bug 3559, 'Opening preferences window causes out of bounds read'”
    
    This reverts commit 0314464dd5c5e4e49f7fb5835e0316726267506e.

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=c7030e74363abe028f4369c0dfc3762829db7b34
Merge: e8971e4 8f65fc9
Author: Colin Leroy <colin@colino.net>
Date:   Mon Nov 16 10:17:01 2015 +0100

    Merge branch 'master' of file:///home/git/claws
Comment 5 Ricardo Mones 2015-11-17 11:08:38 CET
The "Patch to fix out of bounds" breaks preferences window, see bug #3565.
Comment 6 users 2015-11-17 11:13:02 CET
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-17 11:13:02.562978438 +0100
http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=9306c2e76e07c56bd445f75044e3dffee5f2a48d
Merge: 63af23e fee90db
Author: Colin Leroy <colin@colino.net>
Date:   Tue Nov 17 11:13:02 2015 +0100

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

http://git.claws-mail.org/?p=claws.git;a=commitdiff;h=fee90dbf349e4d16a11a4ec66ed21ea7610ce8f1
Author: Colin Leroy <colin@colino.net>
Date:   Tue Nov 17 11:11:56 2015 +0100

    Fix bug #3559 more correctly