D12513: CVE-2018-10361: privilege escalation

[Matthias Gerstner]

mgerstner added a comment.


  Hi,
  
  I am the guy that came up with the initial security report. I contacted
  //cullman// about the issue and we've exchanged a couple of emails about how
  to improve the code.
  
  He asked me about what approach would be better: Setting up the temporary file
  in $TMPDIR and potentially lose the atomic rename() possibility or keeping the
  approach of creating the temporary file in the target directory.
  
  We agreed upon that I add my thoughts here in this Phabricator entry for
  public discussion.
  
  The issue I reported was caused by reopening the temporary file which was
  probably caused by a misunderstanding of the QTemporaryFile API. The new code
  discussed so far should fix this issue and thus the exploit I published.
  
  Apart from this I don't think it matters much if the temporary file is kept in
  $TMPDIR or in the target directory. If the target directory is owned by a
  non-root user then there is always room for shenanigans by the unprivileged
  user. Therefore I would stick to the approach of keeping the temporary file in
  the target directory and additionally to the following:
  
  - enter the target directory via chdir()
  - check if the owner and group of the directory:
      - if owned by root:root, good to go
    - otherwise either reject the operation (simple) or do a temporary privdrop to the owner/group of the directory including drop of  supplementary groups (complex).
  - create the tmpfile in the target dir and do the renameat() using only AT_FDCWD
  - restore privileges, if necessary
  
  The tricky thing is doing the privdrop, which is probably not covered by the
  Qt core library. The good thing about it is that with doing this the kernel
  takes over worrying about permission handling, which it is good at.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D12513

To: cullmann, dfaure
Cc: mgerstner, aacid, ngraham, fvogt, cullmann, #frameworks, michaelh, kevinapavew, bruns, demsking, sars, dhaumann
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180427/80e13ddd/attachment.html>