Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use-after-free #3947

Closed
wants to merge 1 commit into from
Closed

Fix use-after-free #3947

wants to merge 1 commit into from

Conversation

richsalz
Copy link
Contributor

Also fix a RANDerr call.

Fixes #3946

@@ -84,7 +85,8 @@ int opt_rand(int opt)
return loadfiles(opt_arg());
break;
case OPT_R_WRITERAND:
save_rand_file = opt_arg();
OPENSSL_free(save_rand_file);
Copy link
Contributor

Choose a reason for hiding this comment

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

If there is situation when OPENSSL_free would be required here, then it would be appropriate to save_rand_file = NULL after line 69. Otherwise we would be looking at double-free. No?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's only called on app exit so I don't think so, but it is safer to do so. updated commit pushed

Copy link
Contributor

Choose a reason for hiding this comment

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

But you can start openssl without arguments and invoke "apps" or one several times, right?

Also fix a RANDerr call.
@dot-asm dot-asm added the approval: done This pull request has the required number of approvals label Jul 17, 2017
@richsalz
Copy link
Contributor Author

richsalz commented Jul 17, 2017 via email

levitte pushed a commit that referenced this pull request Jul 17, 2017
Also fix a RANDerr call.

Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from #3947)
@richsalz
Copy link
Contributor Author

merged with 5435ba0, thanks!

@richsalz richsalz closed this Jul 17, 2017
@richsalz richsalz deleted the fix-use-after-free branch July 17, 2017 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apps rand use after free.
2 participants