Fix information leak via /tmp
ClosedPublic

Authored by nicolasfella on Aug 5 2017, 11:41 AM.

Details

Summary

BUG: 383144

Diff Detail

Repository
R224 KDE Connect
Lint
Lint Skipped
Unit
Unit Tests Skipped
nicolasfella created this revision.Aug 5 2017, 11:41 AM
apol requested changes to this revision.Aug 5 2017, 3:54 PM
apol added a subscriber: apol.
apol added inline comments.
plugins/notifications/notification.cpp
36

Use QStandardPaths, this isn't portable.

This revision now requires changes to proceed.Aug 5 2017, 3:54 PM
thomasp added a subscriber: thomasp.Aug 5 2017, 5:55 PM
nicolasfella edited edge metadata.

Use QStandardPaths

thomasp added inline comments.Aug 5 2017, 8:27 PM
plugins/notifications/notification.cpp
37

Are the icons reused over restarts? In that case a cache is what you want.
If they are only short lived, TempLocation is a better fit as /tmp is usually a tmpfs(5).
Writing the files to disk just to delete them 5 seconds later does nothing but wear the disk.
To address the privacy concerns, set appropriate permissions on the directory (i.e. only owner should have access)

119

Where is this file deleted?

apol added inline comments.Aug 6 2017, 11:21 PM
plugins/notifications/notification.cpp
117

QUrl destinationUrl = QUrl::fromLocalPath(mIconPath);

And drop the line below

Set file permissions for icon files. Only the owner can read the files now

thomasp added inline comments.Aug 7 2017, 4:22 PM
plugins/notifications/notification.cpp
69

The file is created and filled with a FileTransferJob. Setting the file permission after the job has finished does not help privacy. Please set the permissions on the directory.

nicolasfella planned changes to this revision.Sep 24 2017, 10:06 AM
albertvaka accepted this revision.Sep 24 2017, 6:25 PM
nicolasfella edited the summary of this revision. (Show Details)

Set strict permission on the tmp directory. This way each user needs his owm tmp directory or else only one user could use KDE Connect on a system,
because sharing the tmp folder as before will be impossible.

nicolasfella edited the summary of this revision. (Show Details)Dec 29 2017, 6:48 PM
apol accepted this revision.Dec 29 2017, 8:09 PM
This revision is now accepted and ready to land.Dec 29 2017, 8:09 PM
This revision was automatically updated to reflect the committed changes.