GNOME Bugzilla – Bug 691500
[PATCH] egg-armor: Fix memrchr() call with negative string length
Last modified: 2013-01-13 08:42:46 UTC
Created attachment 233185 [details] [review] [PATCH] egg-armor: Fix memrchr() call with negative string length I could not reproduce this issue reliably as is, but Karsten pointed me to a variant of this bug that could be reproduced easily: gnome-keyring import ~/.ssh/id_rsa See https://bugzilla.redhat.com/show_bug.cgi?id=893162 --- egg/egg-armor.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
This looks good. I'll try to add a test case for it.
Created attachment 233255 [details] [review] Patch with tests and fixes bad practice for git master egg-armor: Fix memrchr() call with negative string length * Initial patch by Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com> * The cause of this bug was reusing argument variables for other purposes in parsing functions when that didn't really make sense, so fix this as well. * Add tests that catch this issue. See https://bugzilla.redhat.com/show_bug.cgi?id=893162
Review of attachment 233255 [details] [review]: I didn't study the actual logic too closely, but it looks right in outline, and the addition of tests greatly increases confidence here.
Review of attachment 233255 [details] [review]: Looks great. And thanks Stef for adding the test case. I'm just wondering if that '&&' on the following line shouldn't be a '||'. if (ARMOR_SUFF_L > len && strncmp ((gchar*)at, ARMOR_SUFF, ARMOR_SUFF_L) != 0)
Created attachment 233260 [details] [review] egg-armor: Handle mismatched but not truncated suffix line * Discovered by Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com>
Attachment 233260 [details] pushed as 583d9cc - egg-armor: Handle mismatched but not truncated suffix line
For distributors back-porting this fix: The original patch is an adequate and minimal fix for the issue.