Skip to content

[Reflection] Sanity-check metadata sizes in MetadataReader before reading. #37863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 14, 2021
Merged

[Reflection] Sanity-check metadata sizes in MetadataReader before reading. #37863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 36 additions & 34 deletions include/swift/Remote/MetadataReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,21 +170,24 @@ class MetadataReader {
using StoredSize = typename Runtime::StoredSize;

private:
/// The maximum number of bytes to read when reading metadata. Anything larger
/// will automatically return failure. This prevents us from reading absurd
/// amounts of data when we encounter corrupt values for sizes/counts.
static const uint64_t MaxMetadataSize = 1048576; // 1MB

/// A cache of built types, keyed by the address of the type.
std::unordered_map<StoredPointer, BuiltType> TypeCache;

using MetadataRef = RemoteRef<TargetMetadata<Runtime>>;
using OwnedMetadataRef =
std::unique_ptr<const TargetMetadata<Runtime>, delete_with_free>;
using MetadataRef = RemoteRef<const TargetMetadata<Runtime>>;
using OwnedMetadataRef = MemoryReader::ReadBytesResult;

/// A cache of read type metadata, keyed by the address of the metadata.
std::unordered_map<StoredPointer, OwnedMetadataRef>
MetadataCache;

using ContextDescriptorRef = RemoteRef<TargetContextDescriptor<Runtime>>;
using OwnedContextDescriptorRef =
std::unique_ptr<const TargetContextDescriptor<Runtime>,
delete_with_free>;
using ContextDescriptorRef =
RemoteRef<const TargetContextDescriptor<Runtime>>;
using OwnedContextDescriptorRef = MemoryReader::ReadBytesResult;

/// A reference to a context descriptor that may be in an unloaded image.
class ParentContextDescriptorRef {
Expand Down Expand Up @@ -984,7 +987,9 @@ class MetadataReader {

auto cached = ContextDescriptorCache.find(address);
if (cached != ContextDescriptorCache.end())
return ContextDescriptorRef(address, cached->second.get());
return ContextDescriptorRef(
address, reinterpret_cast<const TargetContextDescriptor<Runtime> *>(
cached->second.get()));

// Read the flags to figure out how much space we should read.
ContextDescriptorFlags flags;
Expand All @@ -993,9 +998,9 @@ class MetadataReader {
return nullptr;

TypeContextDescriptorFlags typeFlags(flags.getKindSpecificFlags());
unsigned baseSize = 0;
unsigned genericHeaderSize = sizeof(GenericContextDescriptorHeader);
unsigned metadataInitSize = 0;
uint64_t baseSize = 0;
uint64_t genericHeaderSize = sizeof(GenericContextDescriptorHeader);
uint64_t metadataInitSize = 0;
bool hasVTable = false;

auto readMetadataInitSize = [&]() -> unsigned {
Expand Down Expand Up @@ -1059,7 +1064,7 @@ class MetadataReader {
// Determine the full size of the descriptor. This is reimplementing a fair
// bit of TrailingObjects but for out-of-process; maybe there's a way to
// factor the layout stuff out...
unsigned genericsSize = 0;
uint64_t genericsSize = 0;
if (flags.isGeneric()) {
GenericContextDescriptorHeader header;
auto headerAddr = address
Expand All @@ -1077,7 +1082,7 @@ class MetadataReader {
* sizeof(TargetGenericRequirementDescriptor<Runtime>);
}

unsigned vtableSize = 0;
uint64_t vtableSize = 0;
if (hasVTable) {
TargetVTableDescriptorHeader<Runtime> header;
auto headerAddr = address
Expand All @@ -1092,22 +1097,20 @@ class MetadataReader {
vtableSize = sizeof(header)
+ header.VTableSize * sizeof(TargetMethodDescriptor<Runtime>);
}

unsigned size = baseSize + genericsSize + metadataInitSize + vtableSize;
auto buffer = (uint8_t *)malloc(size);
if (buffer == nullptr) {

uint64_t size = baseSize + genericsSize + metadataInitSize + vtableSize;
if (size > MaxMetadataSize)
return nullptr;
}
if (!Reader->readBytes(RemoteAddress(address), buffer, size)) {
free(buffer);
auto readResult = Reader->readBytes(RemoteAddress(address), size);
if (!readResult)
return nullptr;
}

auto descriptor
= reinterpret_cast<TargetContextDescriptor<Runtime> *>(buffer);
auto descriptor =
reinterpret_cast<const TargetContextDescriptor<Runtime> *>(
readResult.get());

ContextDescriptorCache.insert(
std::make_pair(address, OwnedContextDescriptorRef(descriptor)));
std::make_pair(address, std::move(readResult)));
return ContextDescriptorRef(address, descriptor);
}

Expand Down Expand Up @@ -1633,7 +1636,9 @@ class MetadataReader {
MetadataRef readMetadata(StoredPointer address) {
auto cached = MetadataCache.find(address);
if (cached != MetadataCache.end())
return MetadataRef(address, cached->second.get());
return MetadataRef(address,
reinterpret_cast<const TargetMetadata<Runtime> *>(
cached->second.get()));

StoredPointer KindValue = 0;
if (!Reader->readInteger(RemoteAddress(address), &KindValue))
Expand Down Expand Up @@ -1807,18 +1812,15 @@ class MetadataReader {
}

MetadataRef _readMetadata(StoredPointer address, size_t sizeAfter) {
auto size = sizeAfter;
uint8_t *buffer = (uint8_t *) malloc(size);
if (!buffer)
if (sizeAfter > MaxMetadataSize)
return nullptr;

if (!Reader->readBytes(RemoteAddress(address), buffer, size)) {
free(buffer);
auto readResult = Reader->readBytes(RemoteAddress(address), sizeAfter);
if (!readResult)
return nullptr;
}

auto metadata = reinterpret_cast<TargetMetadata<Runtime>*>(buffer);
MetadataCache.insert(std::make_pair(address, OwnedMetadataRef(metadata)));
auto metadata =
reinterpret_cast<const TargetMetadata<Runtime> *>(readResult.get());
MetadataCache.insert(std::make_pair(address, std::move(readResult)));
return MetadataRef(address, metadata);
}

Expand Down
6 changes: 6 additions & 0 deletions test/stdlib/symbol-visibility-linux.test-sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
// RUN: -e _ZNSt6vectorIjSaIjEE13_M_insert_auxIJjEEEvN9__gnu_cxx17__normal_iteratorIPjS1_EEDpOT_ \
// RUN: -e _ZNSt6vectorISt10unique_ptrIKvSt8functionIFvPS1_EEESaIS6_EE19_M_emplace_back_auxIJS6_EEEvDpOT_ \
// RUN: -e _ZNSt6vectorISt10unique_ptrIKvSt8functionIFvPS1_EEESaIS6_EE17_M_realloc_insertIJS6_EEEvN9__gnu_cxx17__normal_iteratorIPS6_S8_EEDpOT_ \
// RUN: -e _ZNSt10_HashtableImSt4pairIKmSt10unique_ptrIKvSt8functionIFvPS3_EEEESaIS9_ENSt8__detail10_Select1stESt8equal_toImESt4hashImENSB_18_Mod_range_hashingENSB_20_Default_ranged_hashENSB_20_Prime_rehash_policyENSB_17_Hashtable_traitsILb0ELb0ELb1EEEE10_M_emplaceIJS0_ImS8_EEEES0_INSB_14_Node_iteratorIS9_Lb0ELb0EEEbESt17integral_constantIbLb1EEDpOT_ \
// RUN: -e _ZNSt10_HashtableImSt4pairIKmSt10unique_ptrIKvSt8functionIFvPS3_EEEESaIS9_ENSt8__detail10_Select1stESt8equal_toImESt4hashImENSB_18_Mod_range_hashingENSB_20_Default_ranged_hashENSB_20_Prime_rehash_policyENSB_17_Hashtable_traitsILb0ELb0ELb1EEEE13_M_rehash_auxEmSt17integral_constantIbLb1EE \
// RUN: -e _ZNSt10_HashtableImSt4pairIKmSt10unique_ptrIKvSt8functionIFvPS3_EEEESaIS9_ENSt8__detail10_Select1stESt8equal_toImESt4hashImENSB_18_Mod_range_hashingENSB_20_Default_ranged_hashENSB_20_Prime_rehash_policyENSB_17_Hashtable_traitsILb0ELb0ELb1EEEED2Ev \
// RUN: -e _ZNSt3_V28__rotateIPcEET_S2_S2_S2_St26random_access_iterator_tag \
// RUN: -e _ZN9__gnu_cxx12__to_xstringINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEcEET_PFiPT0_mPKS8_P13__va_list_tagEmSB_z \
// RUN: -e _ZN9__gnu_cxx12__to_xstringINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEcEET_PFiPT0_mPKS8_St9__va_listEmSB_z \
Expand All @@ -38,6 +41,9 @@
// RUN: -e _ZNSt6vectorIjSaIjEE13_M_insert_auxIJjEEEvN9__gnu_cxx17__normal_iteratorIPjS1_EEDpOT_ \
// RUN: -e _ZNSt6vectorISt10unique_ptrIKvSt8functionIFvPS1_EEESaIS6_EE19_M_emplace_back_auxIJS6_EEEvDpOT_ \
// RUN: -e _ZNSt6vectorISt10unique_ptrIKvSt8functionIFvPS1_EEESaIS6_EE17_M_realloc_insertIJS6_EEEvN9__gnu_cxx17__normal_iteratorIPS6_S8_EEDpOT_ \
// RUN: -e _ZNSt10_HashtableImSt4pairIKmSt10unique_ptrIKvSt8functionIFvPS3_EEEESaIS9_ENSt8__detail10_Select1stESt8equal_toImESt4hashImENSB_18_Mod_range_hashingENSB_20_Default_ranged_hashENSB_20_Prime_rehash_policyENSB_17_Hashtable_traitsILb0ELb0ELb1EEEE10_M_emplaceIJS0_ImS8_EEEES0_INSB_14_Node_iteratorIS9_Lb0ELb0EEEbESt17integral_constantIbLb1EEDpOT_ \
// RUN: -e _ZNSt10_HashtableImSt4pairIKmSt10unique_ptrIKvSt8functionIFvPS3_EEEESaIS9_ENSt8__detail10_Select1stESt8equal_toImESt4hashImENSB_18_Mod_range_hashingENSB_20_Default_ranged_hashENSB_20_Prime_rehash_policyENSB_17_Hashtable_traitsILb0ELb0ELb1EEEE13_M_rehash_auxEmSt17integral_constantIbLb1EE \
// RUN: -e _ZNSt10_HashtableImSt4pairIKmSt10unique_ptrIKvSt8functionIFvPS3_EEEESaIS9_ENSt8__detail10_Select1stESt8equal_toImESt4hashImENSB_18_Mod_range_hashingENSB_20_Default_ranged_hashENSB_20_Prime_rehash_policyENSB_17_Hashtable_traitsILb0ELb0ELb1EEEED2Ev \
// RUN: -e _ZNSt3_V28__rotateIPcEET_S2_S2_S2_St26random_access_iterator_tag \
// RUN: -e _ZN9__gnu_cxx12__to_xstringINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEcEET_PFiPT0_mPKS8_P13__va_list_tagEmSB_z \
// RUN: -e _ZN9__gnu_cxx12__to_xstringINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEcEET_PFiPT0_mPKS8_St9__va_listEmSB_z \
Expand Down