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

Override RAM after exit or logout #196

Open
finn-freitag opened this issue Jan 6, 2025 · 10 comments
Open

Override RAM after exit or logout #196

finn-freitag opened this issue Jan 6, 2025 · 10 comments
Labels
Security To fix this issue is important

Comments

@finn-freitag
Copy link
Collaborator

We should overwrite the RAM where the passwords were stored after the user was automatically logged out or when the program was closed. At the moment, the RAM is only released so that it is overwritten bit by bit. This should happen immediately.

@finn-freitag finn-freitag added the Security To fix this issue is important label Jan 6, 2025
@FrozenAssassine
Copy link
Owner

Any idea?

@Flamifly
Copy link
Contributor

Flamifly commented Jan 6, 2025

You could do it in an unsafe way (which might be end up pretty bad since c# is a managed Language and you would delete the value without it's knowing)

You could delete the Pointers to the value and call the GC to delete it (if it does not have anything that have a pointer to it, it would be deleted)

You could override all Values of the Password Items with random values and just set the Items to null
(with overriding I mean setting the values with =)

I don't know any safe way to overwrite the values bitwise tbh

https://stackoverflow.com/questions/17509891/c-sharp-how-to-completely-remove-object-from-memory

@finn-freitag
Copy link
Collaborator Author

As far as I know, the GC just releases the objects. They aren't removed directly.

@Flamifly
Copy link
Contributor

Flamifly commented Jan 6, 2025

What you mean exactly by "releases"?
I thought the GC would check if an Object is still in use and if it is not the case it will release the memory and delete it

"Reclaims objects that are no longer being used, clears their memory, and keeps the memory available for future allocations. Managed objects automatically get clean content to start with, so their constructors don't have to initialize every data field."
So the GC will clear the Memory if there aint references to the object
https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/fundamentals

Addition thing:
https://stackoverflow.com/questions/39447990/c-sharp-securely-delete-variable-from-memory

@Flamifly
Copy link
Contributor

We should probably to avoid having the Passwords in the Memory in cleartext.

For example we should convert the Passwords After the Decryption to a SecureString instead storing them as strings. (To show the Passwords we could create images instead showing the plain password)

More about that:
https://www.reddit.com/r/csharp/s/N4OYFNw7MI

@FrozenAssassine
Copy link
Owner

I think it does not matter how the password are in RAM when the user is logged in, but on logout we should remove them.

@Flamifly
Copy link
Contributor

It's always better if you try to avoid that someone else can get access to the Data.

For example we try to hide Our Master Password as much as possible (which makes sense to avoid someone get it so he can't get access to EasePass or decrypt the Database)
But if we do not try to avoid someone accessing the actual Password it does not make sense to actual hide the Password.

We try to hide something which is used to get things that could be get by some other Process easiely.

So if we say it doesn't matter while we are logged in we can store the Masterpassword in cleartext too

@Flamifly
Copy link
Contributor

If you just care about to delete the Passwords from the Memory you can do it this way
https://stackoverflow.com/questions/32256138/clear-c-sharp-string-from-memory

by that you will change the characters of the string. The Fixed statement will take care that the GC does not move the String around.
(I personally would prefer changing the Implementation that we do not store the Password in cleartext before doing that)

@FrozenAssassine
Copy link
Owner

But would it not be much simpler to just use SecureString for everything that is a password?

We would not have to clear it from memory directly. In my opinion a much safer and better way?

@Flamifly
Copy link
Contributor

Flamifly commented Jan 16, 2025

Yea that's much better than the current implementation.
One Problem will always exist. We need at some point to get the string of the SecureString. We need that because we want to show the Password to the User.
By that there will exist a string of a Password in the Memory.

To make it overall more secure we need to zero out a string of a Password as fast as possible (when we do not have to show it anymore) and remove the reference to the string (try to let it remove from the Memory by the gc)
(An even safer approach would be to show only an Image of the Password as I mentioned earlier but that would be way harder to implement to still allow copy it etc.)

We should also change the way the Passwords are saved. At the moment we save it as strings, which can be really bad in terms of security (for example remove it from the Memory or overwrite the values in a save situation in c#) .
By that I mean we should probably save it as a char array which is much better to handle on the Memory (the gc handle a char[] way better than a string a string can exist multiple times on the heap and will not removed always if there are no references to it)

This approach will be way more secure than the current one.

But we should still try to remove every Password from the Memory if the user is not logged in.
The SecureString should prevent things to read a string from the Memory but I would say if you know what you are doing you can get access to that still.

If we do not need it we should always remove the Passwords from the Memory even if it should be secure saved (by that I mean clear it and dispose it after)

We should also take care about the clipboard history. (Not sure if that's already done or not)
If we copy the Password to the Clipboard it probably will be saved to the history, which might in addition an security issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Security To fix this issue is important
Projects
None yet
Development

No branches or pull requests

3 participants