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

filenames (within same folder) can be swapped #119

Open
overheadhunter opened this issue Dec 10, 2015 · 6 comments
Open

filenames (within same folder) can be swapped #119

overheadhunter opened this issue Dec 10, 2015 · 6 comments
Assignees
Labels
type:enhancement Improvements on an existing feature type:security-issue Vulnerabilities, leaks, or any other security-related issue

Comments

@overheadhunter
Copy link
Member

overheadhunter commented Dec 10, 2015

Reported by https://twitter.com/tehjh/status/673541838893129729

Attack description

Currently an attacker can move an encrypted file from one folder to another without being noticed (fixed in #128). Furthermore the names of two files (in same folder) can be swapped, i.e. the file content isn't bound to the filename.

This means an attacker could randomly move files around, hoping for the user to accidentally publishing decrypted versions of secret files.

Preconditions

This attack works if

  • the attacker has write access to the storage service
  • the user stores data, that is meant to be published or shared, in the same vault alongside secret data
  • the user publishes content of decrypted directories without checking (e.g. huge, confusing directories)

Evaluation

Severity: High, as data may leak if moving the right data around
Exploitability: Low, as all of the above conditions need to be satisfied and the attacker needs to guess the right files without screwing up the directory structures too much

Fix

This can be solved by adding the directory ID as additional data (see SIV mode) to the filename as well as the filename to the file header MAC.

@overheadhunter overheadhunter added type:enhancement Improvements on an existing feature type:security-issue Vulnerabilities, leaks, or any other security-related issue labels Dec 10, 2015
@ghost
Copy link

ghost commented Dec 26, 2015

Perhaps you could utilise this to keep the file path short?
https://www.boxcryptor.com/en/blog/our-encrypted-filename-encoding-now-open-source
https://github.com/secomba

@overheadhunter
Copy link
Member Author

@deleteIt Very interesting, there is already an unicode based encoding reducing the byte/char ratio. While this allows us to use a simpler directory structure, it doesn't prevent the attack described above. Anyway thanks for the link!

@ghost
Copy link

ghost commented Dec 29, 2015

Oh, I hadn't actually looked at it, I was just thinking that you could use the same idea to encode that data using a similar system.

@overheadhunter
Copy link
Member Author

@deleteIt Don't worry, still an interesting hint ;)

overheadhunter added a commit that referenced this issue Jan 10, 2016
… during filename encryption/decryption

- Using WeakValuedCache in all filesystem layers to prevent "twin" instances of the same folder
- Merge branch 'layered-io' of https://github.com/cryptomator/cryptomator into layered-io
@markuskreusch markuskreusch added this to the 1.0 milestone Feb 26, 2016
@overheadhunter
Copy link
Member Author

As #128 is fixed, it is no longer possible to move files from one directory to another.

@markuskreusch markuskreusch modified the milestones: 1.x, 1.0 Feb 26, 2016
@markuskreusch
Copy link
Contributor

markuskreusch commented Feb 26, 2016

Suggested fix

At the moment the filename is not added to the file header MAC. It would be possible and would solve the problem that contents from any file can be moved ot any other file within the same directory without causing authentication errors.

Adding the filename to the MAC would cause performance and bandwidth issues on mobile devices because the full file has to be retrieved and rewritten to change the header when a file is renamed. To prevent this the authentication information, maybe the full header, can be moved to a metadata file which is placed alongside the file containing the encrypted data. This would allow mobile devices to only read and rewrite a small file instead of the full data. On the other hand that would cause problems if those files would not be synced correctly - currently only the complete file can be synced at once. If authentication data and file contents are split into two files this is no longer true. The application could (sometimes) not tell if data is unauthentic or synchronisation has not finished. Because having many authentication errors of this sort would mean that user get scared we have to think this through a little more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement Improvements on an existing feature type:security-issue Vulnerabilities, leaks, or any other security-related issue
Projects
None yet
Development

No branches or pull requests

3 participants