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

Clear password without memset #182

Closed
wants to merge 1 commit into from

Conversation

ngan
Copy link
Contributor

@ngan ngan commented Apr 10, 2024

Fixes #103
Closes #106

Instead of using memset() to clear the password, clear the password byte by byte in a loop. Additionally, we mark the char* as volatile to ensure that the compiler does not optimize away the memory access or the clearing operation.

@@ -55,7 +55,7 @@ export DISTRIBUTION_SLUG
# and chmod this directory on the host so that the permissions are persisted when the
# the directory is mounted in the containers. Since the mysql container runs as a non-root
# user, we need to ensure that the directory is writable by all users.
mkdir tmp/mysql-certs
mkdir -p tmp/mysql-certs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making this change because it was erring when I run this script locally multiple times.

@ngan
Copy link
Contributor Author

ngan commented May 14, 2024

I talked to @composerinteralia about this at RailsConf and we were unsure why the password is cleared at all. It's still available in Ruby, so why bother clearing it in C? We need the password to stick around since it's needed/used for reconnecting, etc.

@ngan ngan closed this May 14, 2024
@ngan ngan deleted the clear-password branch May 14, 2024 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use memset_s() instead of memset() to clear passwords
1 participant