Skip to content

Fix ccm_memory() cleaning user-supplied key#327

Merged
sjaeckel merged 2 commits intodevelopfrom
fix/ccm_segfault
Nov 20, 2017
Merged

Fix ccm_memory() cleaning user-supplied key#327
sjaeckel merged 2 commits intodevelopfrom
fix/ccm_segfault

Conversation

@sjaeckel
Copy link
Copy Markdown
Member

  • tests are added or updated

This fixes a segfault caused by ccm_memory() with LTC_CLEAN_STACK enabled.

@sjaeckel sjaeckel requested a review from karel-m November 10, 2017 16:04
@sjaeckel sjaeckel added this to the next milestone Nov 10, 2017
@sjaeckel sjaeckel added the bug label Nov 10, 2017
Comment thread src/encauth/ccm/ccm_memory.c Outdated
mask = 0;
zeromem(skey, sizeof(*skey));
if (skey != uskey) {
zeromem(skey, sizeof(*skey));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already if (skey != uskey) couple of lines above (line 334).

Should not it be better to move zeromem(skey, sizeof(*skey)) there?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so you mean like this?

    cipher_descriptor[cipher].done(skey);
#ifdef LTC_CLEAN_STACK
    zeromem(skey,   sizeof(*skey));
#endif

would be fine for me

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's it.

Copy link
Copy Markdown
Member

@karel-m karel-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@sjaeckel sjaeckel merged commit 85ac227 into develop Nov 20, 2017
@sjaeckel sjaeckel deleted the fix/ccm_segfault branch November 20, 2017 13:25
sjaeckel added a commit that referenced this pull request Dec 5, 2017
Fix ccm_memory() cleaning user-supplied key
(cherry picked from commit 85ac227)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants