已开启
fix UnTarFile #2261
dongtingchi创建于 8月22日
fix UnTarFile #2261
已开启
共 3 个文件变更+72-1
| @@ -1355,6 +1355,11 @@ bool BackupExtExtension::CheckIsSplitTarList(const std::vector<const ExtManageIn | |||
| 1355 | // reset untar object | 1355 | // reset untar object |
| 1356 | unSplitTar->Reset(); | 1356 | unSplitTar->Reset(); |
| 1357 | 1357 | ||
| 1358 | + if (!BDir::IsFilePathValid(item.hashName) || !BDir::IsFilePathValid(item.fileName)) { | ||
| 1359 | + HILOGE("Check split tarfile path err, path is forbidden"); | ||
| 1360 | + break; | ||
| 1361 | + } | ||
| 1362 | + | ||
| 1358 | HILOGI("check if split tar, filename: %{public}s, path: %{public}s", GetAnonyPath(item.hashName).c_str(), | 1363 | HILOGI("check if split tar, filename: %{public}s, path: %{public}s", GetAnonyPath(item.hashName).c_str(), |
| 1359 | GetAnonyPath(item.fileName).c_str()); | 1364 | GetAnonyPath(item.fileName).c_str()); |
| 1360 | // check if tar is split | 1365 | // check if tar is split |
| @@ -1401,6 +1406,19 @@ ErrCode BackupExtExtension::RestoreSplitTarListForSpecialCloneCloud(const std::v | |||
| 1401 | // reset untar object | 1406 | // reset untar object |
| 1402 | unSplitTar->Reset(); | 1407 | unSplitTar->Reset(); |
| 1403 | 1408 | ||
| 1409 | + if (!BDir::IsFilePathValid(item.hashName)) { | ||
| 1410 | + HILOGE("Check split tarfile hash path : %{public}s err, path is forbidden", | ||
| 1411 | + GetAnonyPath(item.hashName).c_str()); | ||
| 1412 | + errCode = ERR_INVALID_VALUE; | ||
| 1413 | + break; | ||
| 1414 | + } | ||
| 1415 | + if (!BDir::IsFilePathValid(item.fileName)) { | ||
| 1416 | + HILOGE("Check split tarfile path : %{public}s err, path is forbidden", | ||
| 1417 | + GetAnonyPath(item.fileName).c_str()); | ||
| 1418 | + errCode = ERR_INVALID_VALUE; | ||
| 1419 | + break; | ||
| 1420 | + } | ||
| 1421 | + | ||
| 1404 | // do untar with root path | 1422 | // do untar with root path |
| 1405 | int ret = unSplitTar->UnSplitTar(item.hashName, item.fileName); | 1423 | int ret = unSplitTar->UnSplitTar(item.hashName, item.fileName); |
| 1406 | if (isDebug_ && ret != ERR_OK) { | 1424 | if (isDebug_ && ret != ERR_OK) { |
| @@ -107,6 +107,10 @@ static int GenRealPath(const char *rootPath, const char *pathName, char* &realPa | |||
| 107 | return ERR_PARAM; | 107 | return ERR_PARAM; |
| 108 | } | 108 | } |
| 109 | realPath[allLen] = '\0'; | 109 | realPath[allLen] = '\0'; |
| 110 | + if (!OHOS::FileManagement::Backup::BDir::IsFilePathValid(std::string(realPath))) { | ||
| 111 | + LOGE("GenRealPath path contains forbidden traversal"); | ||
| 112 | + return ERR_PARAM; | ||
| 113 | + } | ||
| 110 | return 0; | 114 | return 0; |
| 111 | } | 115 | } |
| 112 | 116 | ||
| @@ -415,6 +419,10 @@ void UnTarFile::HandleGnuLongLink(ParseTarPath *parseTarPath, bool &isSkip, TarF | |||
| 415 | if (nameLen != fread(parseTarPath->longLink, sizeof(char), nameLen, FilePtr)) { | 419 | if (nameLen != fread(parseTarPath->longLink, sizeof(char), nameLen, FilePtr)) { |
| 416 | free(parseTarPath->longLink); | 420 | free(parseTarPath->longLink); |
| 417 | parseTarPath->longLink = nullptr; | 421 | parseTarPath->longLink = nullptr; |
| 422 | + } else if (!OHOS::FileManagement::Backup::BDir::IsFilePathValid(std::string(parseTarPath->longLink))) { | ||
| 423 | + LOGE("HandleGnuLongLink longLink contains forbidden traversal"); | ||
| 424 | + free(parseTarPath->longLink); | ||
| 425 | + parseTarPath->longLink = nullptr; | ||
| 418 | } | 426 | } |
| 419 | } | 427 | } |
| 420 | 428 | ||
| @@ -438,6 +446,10 @@ void UnTarFile::HandleGnuLongName(ParseTarPath *parseTarPath, bool &isSkip, TarF | |||
| 438 | if (nameLen != fread(parseTarPath->longName, sizeof(char), nameLen, FilePtr)) { | 446 | if (nameLen != fread(parseTarPath->longName, sizeof(char), nameLen, FilePtr)) { |
| 439 | free(parseTarPath->longName); | 447 | free(parseTarPath->longName); |
| 440 | parseTarPath->longName = nullptr; | 448 | parseTarPath->longName = nullptr; |
| 449 | + } else if (!OHOS::FileManagement::Backup::BDir::IsFilePathValid(std::string(parseTarPath->longName))) { | ||
| 450 | + LOGE("HandleGnuLongName longName contains forbidden traversal"); | ||
| 451 | + free(parseTarPath->longName); | ||
| 452 | + parseTarPath->longName = nullptr; | ||
| 441 | } | 453 | } |
| 442 | } | 454 | } |
| 443 | 455 | ||
| @@ -591,7 +603,11 @@ int UnTarFile::ParseTarFile(const char *rootPath, EParseType type) | |||
| 591 | parseTarPath.realName = parseTarPath.longName; | 603 | parseTarPath.realName = parseTarPath.longName; |
| 592 | } | 604 | } |
| 593 | 605 | ||
| 594 | - GenRealPath(rootPath, parseTarPath.realName, parseTarPath.fullPath); | 606 | + if (GenRealPath(rootPath, parseTarPath.realName, parseTarPath.fullPath) != 0) { |
| 607 | + LOGE("GenRealPath failed, invalid path in tar entry"); | ||
| 608 | + FreePointer(&parseTarPath); | ||
| 609 | + return ERR_PARAM; | ||
| 610 | + } | ||
| 595 | parseTarPath.realLink = tarHeader->linkname; | 611 | parseTarPath.realLink = tarHeader->linkname; |
 | |||
| 596 | if (parseTarPath.longLink != nullptr) { | 612 | if (parseTarPath.longLink != nullptr) { |
| 597 | parseTarPath.realLink = parseTarPath.longLink; | 613 | parseTarPath.realLink = parseTarPath.longLink; |
| @@ -366,4 +366,41 @@ HWTEST_F(InstalldUnTarFileTest, Installd_Un_Tar_File_ProcessTarBlock_0100, testi | |||
| 366 | } | 366 | } |
| 367 | GTEST_LOG_(INFO) << "InstalldUnTarFileTest-end Installd_Un_Tar_File_ProcessTarBlock_0100"; | 367 | GTEST_LOG_(INFO) << "InstalldUnTarFileTest-end Installd_Un_Tar_File_ProcessTarBlock_0100"; |
| 368 | } | 368 | } |
| 369 | + | ||
| 370 | +/** | ||
| 371 | + * @tc.number: Installd_Un_Tar_File_GenRealPath_0100 | ||
| 372 | + * @tc.name: Installd_Un_Tar_File_GenRealPath_0100 | ||
| 373 | + * @tc.desc: test ParseTarFile rejects tar entry with path traversal | ||
| 374 | + * @tc.size: MEDIUM | ||
| 375 | + * @tc.type: FUNC | ||
| 376 | + * @tc.level Level 1 | ||
| 377 | + * @tc.require: IC15LE | ||
| 378 | + */ | ||
| 379 | +HWTEST_F(InstalldUnTarFileTest, Installd_Un_Tar_File_GenRealPath_0100, testing::ext::TestSize.Level1) | ||
| 380 | +{ | ||
| 381 | + GTEST_LOG_(INFO) << "InstalldUnTarFileTest-begin Installd_Un_Tar_File_GenRealPath_0100"; | ||
| 382 | + try { | ||
| 383 | + const TestManager tm("Installd_Un_Tar_File_GenRealPath_0100"); | ||
| 384 | + string rootPath = tm.GetRootDirCurTest(); | ||
| 385 | + string testDir = rootPath + "traversal"; | ||
| 386 | + mkdir(testDir.data(), S_IRWXU); | ||
| 387 | + string file = testDir + "/1.txt"; | ||
| 388 | + SaveStringToFile(file, "hello"); | ||
| 389 | + string tarFile = rootPath + "testTraversal.tar"; | ||
| 390 | + | ||
| 391 | + string cmd = "cd " + rootPath + " && tar -cvf testTraversal.tar traversal/../../../etc/passwd 2>/dev/null"; | ||
| 392 | + system(cmd.c_str()); | ||
| 393 | + installd::UnTarFile unTarFile(nullptr); | ||
| 394 | + int ret = unTarFile.UnSplitTar(tarFile, rootPath); | ||
| 395 | + EXPECT_NE(ret, 0); | ||
| 396 | + | ||
| 397 | + // cleanup | ||
| 398 | + std::filesystem::remove_all(testDir); | ||
| 399 | + std::filesystem::remove(tarFile); | ||
| 400 | + } catch (...) { | ||
| 401 | + EXPECT_TRUE(false); | ||
| 402 | + GTEST_LOG_(INFO) << "InstalldUnTarFileTest-an exception occurred by GenRealPath."; | ||
| 403 | + } | ||
| 404 | + GTEST_LOG_(INFO) << "InstalldUnTarFileTest-end Installd_Un_Tar_File_GenRealPath_0100"; | ||
| 405 | +} | ||
| 369 | } // namespace OHOS::FileManagement::Backup | 406 | } // namespace OHOS::FileManagement::Backup |
【安全检视意见|自动扫描】【中危】外部输入校验（路径穿越/符号链接） 本 PR 为 tar 解包新增了 longName/longLink/拼接后 realPath 的 IsFilePathValid 校验，但 SYMTYPE 条目的普通 linkname 字段（tarHeader->linkname，≤100 字节）仍未经校验即赋给 realLink，随后在 ProcessTarBlock 的 SYMTYPE 分支经 CreateSoftlink(realLink, fullPath) 直接创建符号链接。恶意或损坏的备份 tar 可携带指向沙箱外（绝对路径或含 ../）的 linkname：先建立越界软链，再由后续 REGTYPE 条目经 fopen 跟随该软链写文件，构成经典 tar symlink 攻击，绕过本次新增的路径校验（该校验只检查路径字符串，不解析已存在的软链）。 建议：对 realLink 同样调用 BDir::IsFilePathValid 并拒绝绝对路径目标，或在创建软链前用 realpath 确认目标仍位于 rootPath 之内；目标非法时跳过该条目（fail-closed）。