Changeset View
Changeset View
Standalone View
Standalone View
src/kpackage/package.cpp
| Show First 20 Lines • Show All 290 Lines • ▼ Show 20 Line(s) | 289 | { | |||
|---|---|---|---|---|---|
| 291 | // path traversal using symlinks or "../" path segments | 291 | // path traversal using symlinks or "../" path segments | ||
| 292 | 292 | | |||
| 293 | // make sure we got passed a valid path | 293 | // make sure we got passed a valid path | ||
| 294 | Q_ASSERT(QFileInfo(canonicalPath).exists()); | 294 | Q_ASSERT(QFileInfo(canonicalPath).exists()); | ||
| 295 | Q_ASSERT(QFileInfo(canonicalPath).canonicalFilePath() == canonicalPath); | 295 | Q_ASSERT(QFileInfo(canonicalPath).canonicalFilePath() == canonicalPath); | ||
| 296 | // make sure that the base path is also canonical | 296 | // make sure that the base path is also canonical | ||
| 297 | // this was not the case until 5.8, making this check fail e.g. if /home is a symlink | 297 | // this was not the case until 5.8, making this check fail e.g. if /home is a symlink | ||
| 298 | // which in turn would make plasmashell not find the .qml files | 298 | // which in turn would make plasmashell not find the .qml files | ||
| 299 | //installed package | ||||
| 300 | if (tempRoot.isEmpty()) { | ||||
| 299 | Q_ASSERT(QDir(path).exists()); | 301 | Q_ASSERT(QDir(path).exists()); | ||
| 300 | Q_ASSERT(path == QStringLiteral("/") || QDir(path).canonicalPath() + '/' == path); | 302 | Q_ASSERT(path == QStringLiteral("/") || QDir(path).canonicalPath() + '/' == path); | ||
| 303 | | ||||
| 301 | if (canonicalPath.startsWith(path)) { | 304 | if (canonicalPath.startsWith(path)) { | ||
| 302 | return true; | 305 | return true; | ||
| 303 | } | 306 | } | ||
| 307 | //temporary compressed package | ||||
| 308 | } else { | ||||
| 309 | Q_ASSERT(QDir(tempRoot).exists()); | ||||
| 310 | Q_ASSERT(tempRoot == QStringLiteral("/") || QDir(tempRoot).canonicalPath() + '/' == tempRoot); | ||||
| 311 | | ||||
| 312 | if (canonicalPath.startsWith(tempRoot)) { | ||||
| 313 | return true; | ||||
| 314 | } | ||||
| 315 | } | ||||
| 304 | qWarning() << "Path traversal attempt detected:" << canonicalPath << "is not inside" << path; | 316 | qWarning() << "Path traversal attempt detected:" << canonicalPath << "is not inside" << path; | ||
| 305 | return false; | 317 | return false; | ||
| 306 | } | 318 | } | ||
| 307 | 319 | | |||
| 308 | 320 | | |||
| 309 | QString Package::filePath(const QByteArray &fileType, const QString &filename) const | 321 | QString Package::filePath(const QByteArray &fileType, const QString &filename) const | ||
| 310 | { | 322 | { | ||
| 311 | if (!d->valid) { | 323 | if (!d->valid) { | ||
| Show All 30 Lines | |||||
| 342 | } else { | 354 | } else { | ||
| 343 | //when filetype is empty paths is always empty, so try with an empty string | 355 | //when filetype is empty paths is always empty, so try with an empty string | ||
| 344 | paths << QString(); | 356 | paths << QString(); | ||
| 345 | } | 357 | } | ||
| 346 | 358 | | |||
| 347 | //Nested loop, but in the medium case resolves to just one iteration | 359 | //Nested loop, but in the medium case resolves to just one iteration | ||
| 348 | //qDebug() << "prefixes:" << prefixes.count() << prefixes; | 360 | //qDebug() << "prefixes:" << prefixes.count() << prefixes; | ||
| 349 | foreach (const QString &contentsPrefix, d->contentsPrefixPaths) { | 361 | foreach (const QString &contentsPrefix, d->contentsPrefixPaths) { | ||
| 350 | const QString prefix = fileType == "metadata" ? d->path : (d->path + contentsPrefix); | 362 | QString prefix; | ||
| 363 | //We are an installed package | ||||
| 364 | if (d->tempRoot.isEmpty()) { | ||||
| 365 | prefix = fileType == "metadata" ? d->path : (d->path + contentsPrefix); | ||||
| 366 | //We are a compressed package temporarly uncompressed in /tmp | ||||
| 367 | } else { | ||||
| 368 | prefix = fileType == "metadata" ? d->tempRoot : (d->tempRoot + contentsPrefix); | ||||
| 369 | } | ||||
hein: Could you rewrite this for readability a bit? Nested ternary operators just to use a diff var… | |||||
| 351 | 370 | | |||
| 352 | foreach (const QString &path, paths) { | 371 | foreach (const QString &path, paths) { | ||
| 353 | QString file = prefix + path; | 372 | QString file = prefix + path; | ||
| 354 | 373 | | |||
| 355 | if (!filename.isEmpty()) { | 374 | if (!filename.isEmpty()) { | ||
| 356 | file.append("/").append(filename); | 375 | file.append("/").append(filename); | ||
| 357 | } | 376 | } | ||
| 358 | 377 | | |||
| ▲ Show 20 Lines • Show All 631 Lines • Show Last 20 Lines | |||||
Could you rewrite this for readability a bit? Nested ternary operators just to use a diff var name is ugly, we have implicitly shared strings :)