1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QFileSystemEngine::tempPath: bypass QDir and go straight to QFSEngine

Browse Source 
Temporary paths coming from the environment must be real filesystem
things, never a Qt file engine, so we don't need to create QDir with its
QDirPrivate, in order to call QFileSystemEngine.

The replacing of canonicalPath() with QFSE::absoluteName() is fine
because canonicalizing *after* cleanPath() is the wrong thing. For
example, if you had:

  $ ln -s $HOME/tmp /tmp/symlink
  $ TMPDIR=/tmp/symlink/..
then
  cleanPath($TMPDIR) = /tmp
  absolute($TMPDIR) = /tmp    # QFSE::absoluteName calls cleanPath
  canonical($TMPDIR) = $HOME
  canonical(cleanPath($TMPDIR)) = /tmp

The lack of canonicalization now only affects when the final path is a
symlink. Doing so bought us little security if it is a symlink and it
could change, because the result is not cached and could change from
call to call. That changing is probably worse than any attack, because
you could end up with

  QDir::tempPath() != QDir::tempPath()

[ChangeLog][QtCore][QDir] tempPath() may now return a non-canonical
path. This means going up from it (cdUp()) may result in different paths
from string manipulation (adding "/..").

Pick-to: 6.10 6.9
Change-Id: Iddf6f46edf6f3b6c3222fffd1e1e5479f0be92a9
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
This commit is contained in:
Thiago Macieira 2025-05-08 11:01:39 +02:00 committed by Ahmad Samir
parent 05f8abc61d
commit 7bd7df5aa1
2 changed files with 35 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/corelib/io/qfilesystemengine_unix.cpp
View File

@ -1858,6 +1858,15 @@ QString QFileSystemEngine::rootPath()
return u"/"_s;
}
static constexpr QLatin1StringView nativeTempPath() noexcept
{
// _PATH_TMP usually ends in '/' and we don't want that
QLatin1StringView temp = _PATH_TMP ""_L1;
static_assert(_PATH_TMP[0] == '/', "_PATH_TMP needs to be absolute");
static_assert(_PATH_TMP[1] != '\0', "Are you really sure _PATH_TMP should be the root dir??");
return temp;
}
QString QFileSystemEngine::tempPath()
{
#ifdef QT_UNIX_TEMP_PATH_OVERRIDE
@ -1871,10 +1880,17 @@ QString QFileSystemEngine::tempPath()
temp = QString::fromCFString((CFStringRef)nsPath);
#endif
} else {
temp = _PATH_TMP ""_L1;
constexpr auto nativeTemp = nativeTempPath();
temp = nativeTemp;
}
}
return QDir(QDir::cleanPath(temp)).canonicalPath();
// the environment variable may also end in '/'
if (temp.size() > 1 && temp.endsWith(u'/'))
temp.chop(1);
QFileSystemEntry e(temp, QFileSystemEntry::FromInternalPath{});
return QFileSystemEngine::absoluteName(e).filePath();
#endif
}

29
tests/auto/corelib/io/qdir/tst_qdir.cpp
View File

@ -1240,7 +1240,8 @@ void tst_QDir::current()
#if defined(Q_OS_WIN)
QCOMPARE(newCurrent.absolutePath().toLower(), currentDir.toLower());
#else
QCOMPARE(newCurrent.absolutePath(), currentDir);
// getcwd(2) on Unix returns the canonical path
QCOMPARE(newCurrent.absolutePath(), QDir(currentDir).canonicalPath());
#endif
}
@ -1254,21 +1255,25 @@ void tst_QDir::cd_data()
QTest::addColumn<bool>("successExpected");
QTest::addColumn<QString>("newDir");
int index = m_dataPath.lastIndexOf(QLatin1Char('/'));
QTest::newRow("cdUp") << m_dataPath << ".." << true << m_dataPath.left(index==0?1:index);
// use the canonical path for m_dataPath here, because if TMPDIR points to
// a symlink like what happens on Apple systems (/tmp -> /private/tmp),
// then /tmp/.. will not be the same as / (it's /private).
QString canonicalPath = QDir(m_dataPath).canonicalPath();
int index = canonicalPath.lastIndexOf(QLatin1Char('/'));
QTest::newRow("cdUp") << canonicalPath << ".." << true << canonicalPath.left(index==0?1:index);
QTest::newRow("cdUp non existent (relative dir)") << "anonexistingDir" << ".."
<< true << m_dataPath;
QTest::newRow("cdUp non existent (absolute dir)") << m_dataPath + "/anonexistingDir" << ".."
<< true << m_dataPath;
QTest::newRow("noChange") << m_dataPath << "." << true << m_dataPath;
<< true << canonicalPath;
QTest::newRow("cdUp non existent (absolute dir)") << canonicalPath + "/anonexistingDir" << ".."
<< true << canonicalPath;
QTest::newRow("noChange") << canonicalPath << "." << true << canonicalPath;
#if defined(Q_OS_WIN) // on windows QDir::root() is usually c:/ but cd "/" will not force it to be root
QTest::newRow("absolute") << m_dataPath << "/" << true << "/";
QTest::newRow("absolute") << canonicalPath << "/" << true << "/";
#else
QTest::newRow("absolute") << m_dataPath << "/" << true << QDir::root().absolutePath();
QTest::newRow("absolute") << canonicalPath << "/" << true << QDir::root().absolutePath();
#endif
QTest::newRow("non existant") << "." << "../anonexistingdir" << false << m_dataPath;
QTest::newRow("self") << "." << (QString("../") + QFileInfo(m_dataPath).fileName()) << true << m_dataPath;
QTest::newRow("file") << "." << "qdir.pro" << false << m_dataPath;
QTest::newRow("non existant") << "." << "../anonexistingdir" << false << canonicalPath;
QTest::newRow("self") << "." << (QString("../") + QFileInfo(canonicalPath).fileName()) << true << canonicalPath;
QTest::newRow("file") << "." << "qdir.pro" << false << canonicalPath;
}
void tst_QDir::cd()