Skip to content

Commit 2f31d26

Browse files
committed
Fix PKG extraction path traversal
PKG entry names could contain traversal components and escape the installation directory. Reject absolute and special components, then require the resolved target to remain below the installation root before creating or writing files.
1 parent 0514f52 commit 2f31d26

1 file changed

Lines changed: 59 additions & 9 deletions

File tree

rpcs3/Crypto/unpkg.cpp

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,25 @@ bool package_reader::fill_data(std::map<std::string, install_entry*>& all_instal
877877
return false;
878878
}
879879

880+
std::error_code path_ec;
881+
auto install_path = std::filesystem::weakly_canonical(m_install_path, path_ec);
882+
if (path_ec)
883+
{
884+
pkg_log.warning("Failed to canonicalize installation path '%s' (%s); falling back to lexical normalization.", m_install_path, path_ec.message());
885+
install_path = std::filesystem::path(m_install_path).lexically_normal();
886+
}
887+
888+
if (install_path.empty())
889+
{
890+
pkg_log.error("Failed to normalize installation path for '%s'", m_install_path);
891+
return false;
892+
}
893+
894+
const auto is_inside_install_path = [&install_path](const std::filesystem::path& path)
895+
{
896+
return std::mismatch(install_path.begin(), install_path.end(), path.begin(), path.end()).first == install_path.end();
897+
};
898+
880899
m_install_entries.clear();
881900
m_bootable_file_path.clear();
882901
m_entry_indexer = 0;
@@ -914,7 +933,46 @@ bool package_reader::fill_data(std::map<std::string, install_entry*>& all_instal
914933

915934
std::string_view name = fmt::trim_back_sv(name_buf, "\0"sv);
916935

936+
const std::filesystem::path entry_path{name};
937+
if (entry_path.is_absolute())
938+
{
939+
num_failures++;
940+
pkg_log.error("PKG entry path is absolute: '%s'", name);
941+
break;
942+
}
943+
944+
for (const auto& component : entry_path)
945+
{
946+
if (component == "." || component == "..")
947+
{
948+
num_failures++;
949+
pkg_log.error("PKG entry path contains a special component: '%s'", name);
950+
break;
951+
}
952+
}
953+
954+
if (num_failures)
955+
{
956+
break;
957+
}
958+
917959
std::string path = m_install_path + vfs::escape(name);
960+
path_ec.clear();
961+
auto canonical_path = std::filesystem::weakly_canonical(path, path_ec);
962+
if (path_ec)
963+
{
964+
pkg_log.warning("Failed to canonicalize package path '%s' (%s); falling back to lexical normalization.", path, path_ec.message());
965+
canonical_path = std::filesystem::path(path).lexically_normal();
966+
}
967+
968+
if (canonical_path.empty() || !is_inside_install_path(canonical_path))
969+
{
970+
num_failures++;
971+
pkg_log.error("PKG entry path escapes installation directory: '%s'", name);
972+
break;
973+
}
974+
975+
path = canonical_path.string();
918976

919977
if (entry.pad || (entry.type & ~PKG_FILE_ENTRY_KNOWN_BITS))
920978
{
@@ -952,15 +1010,7 @@ bool package_reader::fill_data(std::map<std::string, install_entry*>& all_instal
9521010
default:
9531011
{
9541012
// TODO: check for valid utf8 characters
955-
const std::string true_path = std::filesystem::path(path).lexically_normal().string();
956-
if (true_path.empty())
957-
{
958-
num_failures++;
959-
pkg_log.error("Failed to normalize package path for '%s'", path);
960-
break;
961-
}
962-
963-
auto map_ptr = &*all_install_entries.try_emplace(true_path).first;
1013+
auto map_ptr = &*all_install_entries.try_emplace(path).first;
9641014

9651015
m_install_entries.push_back({
9661016
.weak_reference = map_ptr,

0 commit comments

Comments
 (0)