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

[security] Unsafe handling of temporary files when using external editor #17

Closed
carnil opened this issue May 19, 2013 · 2 comments
Closed

Comments

@carnil
Copy link
Contributor

carnil commented May 19, 2013

Hi

Looking at the code it looks that when using external editor nodau handles temporary files in unsafe way due to using /tmp/nodau.$time containing unix time. This might lead to overwrite of files owned by a user or even disclosure of information (e.g. a user editing (encrypting) notes).

(Note: For easy reproducing you need to set fs.protected_symlinks=0 on recent Linux kernels[0].)

https://github.com/darkrose/nodau/blob/master/src/edit.c#L159

  1. Evil user creates a world writable file in e.g. /tmp and then creates lots of symlinks /tmp/nodau.$timestamps (with timestamps in future) to this file.
  2. regular user now creates with nodau encrypt testnote a new note.
  3. Evil user now has access to secret note in /tmp/worldwritable after user has saved the file in editor (nodau prompts that the file get's saved

Another usecase would be that if evil user knows the filename writable by the user can make the user overwrite it with note content.

Furthermore in the case how it is implemented right now, depending on the umask of the user editing a note and having the editor open, every other user might read the temporary file during the user having the editor open.

nodau should not use /tmp in unsafe way. Kurt Seifried from Red Hat Security Response Team wrote a nice blog entry describing how to create safely files in various programming languages[1].

[0] https://lwn.net/Articles/503660/
[1] http://kurt.seifried.org/2012/03/14/creating-temporary-files-securely/

Regards,
Salvatore

@TicklishHoneyBee
Copy link
Owner

Salvatore,

when you get a chance, could you let me know if the changes fix this? I'm not 100% sure it does.

Ta

@carnil
Copy link
Contributor Author

carnil commented Jun 8, 2013

Hi Lisa

Looks good, using mkstemp.

Regards,
Salvatore

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

No branches or pull requests

2 participants