Create Task
Maniphest T8075

Fix security issues with KAuth support in KIO
Closed, ResolvedPublic
Actions

Description

KAuth support in KIO is currently disabled due to following security issues:

  • 1. The privilege is persistent for the entire session.
  • 2. The confirmation prompt for the kauth action use does not tell what is going to happen. So you might open a file dialog and then instead of opening a file, write to /bin/sh.
  • 3. Trivial stack-based buffer overflow in the kauth helper: https://cgit.kde.org/kio.git/tree/src/ioslaves/file/sharefd_p.h#n57
  • 4. The socket used to send and receive file descriptors does not have any kind of permission check.
  • 5. Having KIO::Job show a prompt achieves nothing. An application can easily bypass it.

Possible solutions:
Issue 1
Try to revoke authorization of slave and if unsuccessful delete the slave in klauncher.
D10818 D10822 D10437: Store authorization status
D10820 : Send authorization status
D10641 : Revoke authorization
D10824 : Delete slave if revoking wasn't done or unsuccessful

Issue 2
D21782 D21783

Issue 3
Don't use strcpy.
D10273 : Create proper socket address structure

Issue 4
Create socket in user's runtime directory and accept file descriptor only form root process.
D10410 : Secure socket to app connection
D10409 D10411 : Secures app to socket connection

Issue 5
Show prompt from slave's side
D10567 D10568 : Handle prompt in KIO slave

Edit:
Some problems with ktexteditor's privilege escalation, and possible improvements are mentioned here:
https://bugzilla.suse.com/show_bug.cgi?id=1033055#c13

Listed below are some of those improvements that (I think) should be made in KIO as well:

  • 6. Improve message in polkit prompt. D21795
  • 7. Don't elevate privilege if the directory is read-only and user is the owner. D14464
  • 8. If owner of target directory is not root then drop privileges (rejecting the whole operation might be impractical). D14467

Details

Differential Revisions
D21783: Show more details in warning dialog shown before starting a privileged operation
D21795: [KAuth] Add support for action details in Polkit1 backend.

Related Objects
Search...

chinmoyr created this task.Feb 25 2018, 11:44 AM
chinmoyr triaged this task as High priority.
ngraham updated the task description. (Show Details)Apr 4 2018, 8:49 PM
fvogt added a comment.Apr 5 2018, 6:55 AM

KAuth support in KIO is currently disabled due to following security issues:

I just noticed that the KAuth integration in kio isn't actually disabled at all - kio itself doesn't use it anymore but anything else still can. Intentional?

chinmoyr added a comment.Apr 5 2018, 7:49 AM

Not at all intentional. It just didn't crossed my mind. I suppose the cmake file needs to be patched to not build the helper?

fvogt added a comment.Apr 5 2018, 7:51 AM

Not at all intentional. It just didn't crossed my mind. I suppose the cmake file needs to be patched to not build the helper?

Yup, or just not install it.

lbeltrame added a comment.Apr 24 2018, 12:20 PM

The security review by the SUSE security team of an analogous change in ktexteditor highlighted some problems there. @chinmoyr, you may want to look the review up for reference:

https://bugzilla.suse.com/show_bug.cgi?id=1033055#c13

chinmoyr updated the task description. (Show Details)Jun 1 2018, 12:20 PM
chinmoyr updated the task description. (Show Details)Jun 1 2018, 12:44 PM
ngraham added a subscriber: sharvey.Jun 1 2018, 12:56 PM

+ @sharvey, who has been interested in improving the authorization dialog.

mati865 added a subscriber: mati865.Jun 3 2018, 1:52 PM
ngraham added a comment.Jun 4 2018, 12:58 AM

So what more is left to do here? Just improve the UI of the authorization dialog?

mreeves added a subscriber: mreeves.Jun 4 2018, 3:49 AM
chinmoyr added a comment.Jun 7 2018, 11:55 AM
In T8075#146020, @ngraham wrote:

So what more is left to do here? Just improve the UI of the authorization dialog?

Yes and the warning message shown prior to this dialog is quite generic so that needs improvement as well.

ngraham added a comment.Jun 7 2018, 7:35 PM

Thanks. If you need any design help for that last task, please feel free to reach out to VDG. We'd be happy to work with you on it!

cfeck added a subscriber: cfeck.Jun 8 2018, 9:32 PM
kpiwowarski added a subscriber: kpiwowarski.Jan 29 2019, 12:18 AM

What is status of that? ;)

ngraham added a comment.EditedJan 29 2019, 4:57 AM

What needs to be done to push this forward? Do you need any help?

chinmoyr added a comment.Jan 29 2019, 6:42 PM
In T8075#174716, @ngraham wrote:

What needs to be done to push this forward? Do you need any help?

Descriptive warning and privilege drop or job cancellation based on target's ownership. The latter part is up for review while some help is required for the warning stuff.

davidedmundson added a subscriber: davidedmundson.Jan 30 2019, 1:20 AM
  1. Improve message in polkit prompt (issue no. 2). BTW how do I fix this?

You can put placeholders in the message and pass them as annotations to the description.
I'm not sure this exists in Kauth which adds some layers on top. If it doesn't it needs adding

chinmoyr updated the task description. (Show Details)Jun 13 2019, 6:21 PM
chinmoyr updated the task description. (Show Details)Jun 14 2019, 6:14 AM
lbeltrame added a comment.Jul 5 2019, 7:03 PM

For the record, the SUSE security team said (excerpt from http://bugzilla.suse.com/show_bug.cgi?id=1062040#c9):

So I think the early review has been fulfilled by now.

Once the two remaining diffs are closed (for showing what is going on, one being accepted, the other with changes planned) I think the switch can be flipped back to on. It might be worth timing this after a Frameworks release, so that there's plenty of testing in the following month.

ngraham updated the task description. (Show Details)Jul 18 2019, 2:38 PM
nerdopolist added subscribers: feverfew, nerdopolist.Mar 2 2020, 2:57 AM

I think thanks to @feverfew 's work on getting that last patch merged, that the final items on here are complete now?

lbeltrame added a comment.Mar 2 2020, 5:33 AM

I'm going to re-request a look from the SUSE security team.

ngraham updated the task description. (Show Details)Mar 3 2020, 4:40 PM
ngraham closed this task as Resolved.Apr 10 2020, 2:27 AM

Is there an update on that security review? I'm starting to think we should turn this on now and respond to any newly-discovered issues as they come up or else we'll never get it shipped...

Either way, this task is finished, at least as originally described.

lbeltrame added a comment.Apr 10 2020, 6:32 AM

I have asked, however I don't know when they'll be able to look at it.