<table><tr><td style="">mgerstner added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D12513">View Revision</a></tr></table><br /><div><div><p>Hi,</p>

<p>I am the guy that came up with the initial security report. I contacted<br />
<em>cullman</em> about the issue and we've exchanged a couple of emails about how<br />
to improve the code.</p>

<p>He asked me about what approach would be better: Setting up the temporary file<br />
in $TMPDIR and potentially lose the atomic rename() possibility or keeping the<br />
approach of creating the temporary file in the target directory.</p>

<p>We agreed upon that I add my thoughts here in this Phabricator entry for<br />
public discussion.</p>

<p>The issue I reported was caused by reopening the temporary file which was<br />
probably caused by a misunderstanding of the QTemporaryFile API. The new code<br />
discussed so far should fix this issue and thus the exploit I published.</p>

<p>Apart from this I don't think it matters much if the temporary file is kept in<br />
$TMPDIR or in the target directory. If the target directory is owned by a<br />
non-root user then there is always room for shenanigans by the unprivileged<br />
user. Therefore I would stick to the approach of keeping the temporary file in<br />
the target directory and additionally to the following:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">enter the target directory via chdir()</li>
<li class="remarkup-list-item">check if the owner and group of the directory:<ul class="remarkup-list">
<li class="remarkup-list-item phantom-item"><ul class="remarkup-list">
<li class="remarkup-list-item">if owned by root:root, good to go</li>
</ul></li>
<li class="remarkup-list-item">otherwise either reject the operation (simple) or do a temporary privdrop to the owner/group of the directory including drop of  supplementary groups (complex).</li>
</ul></li>
<li class="remarkup-list-item">create the tmpfile in the target dir and do the renameat() using only AT_FDCWD</li>
<li class="remarkup-list-item">restore privileges, if necessary</li>
</ul>

<p>The tricky thing is doing the privdrop, which is probably not covered by the<br />
Qt core library. The good thing about it is that with doing this the kernel<br />
takes over worrying about permission handling, which it is good at.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R39 KTextEditor</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D12513">https://phabricator.kde.org/D12513</a></div></div><br /><div><strong>To: </strong>cullmann, dfaure<br /><strong>Cc: </strong>mgerstner, aacid, ngraham, fvogt, cullmann, Frameworks, michaelh, kevinapavew, bruns, demsking, sars, dhaumann<br /></div>