diff --git a/Core/ELF/ElfReader.cpp b/Core/ELF/ElfReader.cpp index 770bc7f155..4efc8cda6c 100644 --- a/Core/ELF/ElfReader.cpp +++ b/Core/ELF/ElfReader.cpp @@ -15,10 +15,7 @@ // Official git repository and contact information can be found at // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. -#include - #include "Common/StringUtils.h" -#include "Common/Thread/ParallelLoop.h" #include "Common/File/DirListing.h" #include "Common/File/FileUtil.h" @@ -56,7 +53,9 @@ void addrToHiLo(u32 addr, u16 &hi, s16 &lo) lo = (addr & 0xFFFF); u32 naddr = addr - lo; hi = naddr>>16; - u32 test = (hi<<16) + lo; + // Note the casts: hi is a u16, so it promotes to int, and kernel modules load at 0x88000000 - + // shifting a value of 0x8800 left by 16 would overflow a signed int. + u32 test = ((u32)hi << 16) + (u32)lo; if (test != addr) { WARN_LOG_REPORT(Log::Loader, "HI16/LO16 relocation failure?"); @@ -68,8 +67,7 @@ bool ElfReader::LoadRelocations(const Elf32_Rel *rels, int numRelocs) { relocOps.resize(numRelocs); DEBUG_LOG(Log::Loader, "Loading %i relocations...", numRelocs); - std::atomic numErrors; - numErrors.store(0); + int numErrors = 0; { for (int r = 0; r < numRelocs; r++) { @@ -161,7 +159,12 @@ bool ElfReader::LoadRelocations(const Elf32_Rel *rels, int numRelocs) { if (t_type == R_MIPS_HI16) continue; - u32 corrLoAddr = rels[t].r_offset + segmentVAddr[readwrite]; + // The candidate LO16 declares its own segment - use that rather than the HI16's, + // which is what the mismatch warning further down is there to detect. + int t_readwrite = (rels[t].r_info >> 8) & 0xff; + if (t_readwrite >= (int)ARRAY_SIZE(segmentVAddr)) + continue; + u32 corrLoAddr = rels[t].r_offset + segmentVAddr[t_readwrite]; // In MotorStorm: Arctic Edge (US), these are sometimes R_MIPS_16 (instead of LO16.) // It appears the PSP takes any relocation that is not a HI16. @@ -194,10 +197,14 @@ bool ElfReader::LoadRelocations(const Elf32_Rel *rels, int numRelocs) { ERROR_LOG(Log::Loader, "Bad corrLoAddr %08x", corrLoAddr); } } - if (!found) { + if (found) { + op = (op & 0xFFFF0000) | hi; + } else { + // Leave the instruction alone rather than writing hi's initial 0 into it. We + // have no idea what the right immediate is, and zeroing the lui of a lui/addiu + // pair is a guess that's wrong in a way that's hard to trace back to here. ERROR_LOG_REPORT(Log::Loader, "R_MIPS_HI16: could not find R_MIPS_LO16 (r=%d of %d, addr=%08x)", r, numRelocs, addr); } - op = (op & 0xFFFF0000) | hi; } break; @@ -239,7 +246,7 @@ bool ElfReader::LoadRelocations(const Elf32_Rel *rels, int numRelocs) { } if (numErrors) { - WARN_LOG(Log::Loader, "%i bad relocations found!!!", numErrors.load()); + WARN_LOG(Log::Loader, "%i bad relocations found!!!", numErrors); } return numErrors == 0; } @@ -263,8 +270,26 @@ void ElfReader::LoadRelocations2(int rel_seg) ERROR_LOG_REPORT(Log::Loader, "Rel2 segment invalid"); return; } + // GetSegmentPtr only vouches for where the segment starts - p_filesz comes from the file too. + if ((size_t)ph->p_offset + ph->p_filesz > size_) { + ERROR_LOG_REPORT(Log::Loader, "Rel2 segment extends past the end of the file"); + return; + } end = buf+ph->p_filesz; + // Everything below reads forward from buf, so check there's something there each time. All of + // these sizes and indexes come out of the file. + auto haveBytes = [&buf, &end](int n) -> bool { + if (end - buf < n) { + ERROR_LOG_REPORT(Log::Loader, "Rel2: truncated relocation data"); + return false; + } + return true; + }; + + if (!haveBytes(4)) + return; + flag_bits = buf[2]; type_bits = buf[3]; @@ -274,22 +299,40 @@ void ElfReader::LoadRelocations2(int rel_seg) buf += 4; + // Both tables are prefixed by their own size, and are indexed by bitfields out of the command + // words below - so the tables and the indexes into them both need checking. + if (!haveBytes(1)) + return; flag_table = buf; flag_table_size = flag_table[0]; + if (!haveBytes(flag_table_size)) + return; buf += flag_table_size; + if (!haveBytes(1)) + return; type_table = buf; type_table_size = type_table[0]; + if (!haveBytes(type_table_size)) + return; buf += type_table_size; rel_base = 0; last_type = -1; while(buf>(16-flag_bits))&0xffff; + if (flag >= flag_table_size) { + ERROR_LOG_REPORT(Log::Loader, "Rel2: flag %d out of range (table has %d)", flag, flag_table_size); + return; + } flag = flag_table[flag]; seg = (cmd<<(16-seg_bits-flag_bits))&0xffff; @@ -297,6 +340,10 @@ void ElfReader::LoadRelocations2(int rel_seg) type = ( cmd<<(16-type_bits-seg_bits-flag_bits))&0xffff; type = (type>>(16-type_bits))&0xffff; + if (type >= type_table_size) { + ERROR_LOG_REPORT(Log::Loader, "Rel2: type %d out of range (table has %d)", type, type_table_size); + return; + } type = type_table[type]; if((flag&0x01)==0){ @@ -304,6 +351,8 @@ void ElfReader::LoadRelocations2(int rel_seg) if((flag&0x06)==0){ rel_base = cmd>>(seg_bits+flag_bits); }else if((flag&0x06)==4){ + if (!haveBytes(4)) + return; rel_base = buf[0] | (buf[1]<<8) | (buf[2]<<16) | (buf[3]<<24); buf += 4; }else{ @@ -333,17 +382,25 @@ void ElfReader::LoadRelocations2(int rel_seg) if(cmd&0x8000) rel_offset |= 0xffff0000; rel_offset >>= type_bits+seg_bits+flag_bits; + if (!haveBytes(2)) + return; rel_offset = (rel_offset<<16) | (buf[0]) | (buf[1]<<8); buf += 2; rel_base += rel_offset; }else if((flag&0x06)==0x04){ + if (!haveBytes(4)) + return; rel_base = buf[0] | (buf[1]<<8) | (buf[2]<<16) | (buf[3]<<24); buf += 4; }else{ ERROR_LOG_REPORT(Log::Loader, "Rel2: invalid relocat size flag! %x", flag); } - + // seg is seg_bits wide, which can address more segments than we can record. + if (off_seg >= (int)ARRAY_SIZE(segmentVAddr)) { + ERROR_LOG_REPORT(Log::Loader, "Rel2: bad offset segment %d", off_seg); + continue; + } rel_offset = rel_base+segmentVAddr[off_seg]; if (!Memory::IsValidAddress(rel_offset)) { ERROR_LOG_REPORT(Log::Loader, "ELF: Bad rel_offset: %08x", rel_offset); @@ -356,6 +413,8 @@ void ElfReader::LoadRelocations2(int rel_seg) if(last_type!=0x04) lo16 = 0; }else if((flag&0x38)==0x10){ + if (!haveBytes(2)) + return; lo16 = (buf[0]) | (buf[1]<<8); if(lo16&0x8000) lo16 |= 0xffff0000; @@ -438,6 +497,14 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) { return SCE_KERNEL_ERROR_MEMBLOCK_ALLOC_FAILED; } + // e_phnum is a u16, but we can only record the load address of ARRAY_SIZE(segmentVAddr) segments, + // and the relocation code can't refer to segments beyond that either (see LoadRelocations). Real + // PSP modules have a handful - PSP_Header::nsegments is a u8 and no more than 4 are ever used. + if (GetNumSegments() > (int)ARRAY_SIZE(segmentVAddr)) { + ERROR_LOG(Log::Loader, "ELF has %d segments, we support at most %d", GetNumSegments(), (int)ARRAY_SIZE(segmentVAddr)); + return SCE_KERNEL_ERROR_MEMBLOCK_ALLOC_FAILED; + } + // e_ident[EI_VERSION] is ignored // Should we relocate? @@ -467,9 +534,11 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) { entryPoint = header->e_entry; u32 totalStart = 0xFFFFFFFF; u32 totalEnd = 0; + int numLoadSegments = 0; for (int i = 0; i < header->e_phnum; i++) { const Elf32_Phdr *p = &segments[i]; if (p->p_type == PT_LOAD) { + numLoadSegments++; if (p->p_vaddr < totalStart) { totalStart = p->p_vaddr; firstSegAlign = p->p_align; @@ -478,6 +547,12 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) { totalEnd = p->p_vaddr + p->p_memsz; } } + // Without this, totalStart stays 0xFFFFFFFF and totalEnd 0, so totalSize would come out as 1 + // and we'd go on to allocate at 0xFFFFFFFF. + if (numLoadSegments == 0) { + ERROR_LOG(Log::Loader, "ELF has no loadable segments"); + return SCE_KERNEL_ERROR_MEMBLOCK_ALLOC_FAILED; + } totalSize = totalEnd - totalStart; // If a load address is specified that's in regular RAM, override kernel module status @@ -628,7 +703,7 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) { { //We have a relocation table! int sectionToModify = s->sh_info; - if (sectionToModify >= 0) + if (sectionToModify >= 0 && sectionToModify < GetNumSections()) { if (!(sections[sectionToModify].sh_flags & SHF_ALLOC)) { @@ -663,7 +738,7 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) { { //We have a relocation table! int sectionToModify = s->sh_info; - if (sectionToModify >= 0) + if (sectionToModify >= 0 && sectionToModify < GetNumSections()) { if (!(sections[sectionToModify].sh_flags & SHF_ALLOC)) { @@ -788,10 +863,15 @@ bool ElfReader::LoadSymbols() u32 symtabOffset = GetSectionDataOffset(sec); int numSymbols = sections[sec].sh_size / sizeof(Elf32_Sym); - if (!stringBase || !symtab || symtabOffset + sections[sec].sh_size > size_) { + if (!stringBase || !symtab || (size_t)symtabOffset + sections[sec].sh_size > size_) { ERROR_LOG(Log::Loader, "Symbols truncated - ignoring"); return false; } + // Relocating a symbol needs the section addresses LoadInto computed. + if (bRelocate && !sectionAddrs) { + ERROR_LOG(Log::Loader, "LoadSymbols called before LoadInto - ignoring"); + return false; + } for (int sym = 0; sym= size_) + continue; const char *name = stringBase + symtab[sym].st_name; - if (stringOffset + symtab[sym].st_name >= size_) + // And make sure it's terminated inside the file, before anything strlen()s it. + if (strnlen(name, size_ - nameOffset) == size_ - nameOffset) continue; - if (bRelocate) + if (bRelocate) { + // st_shndx is a u16 that can hold reserved values rather than a section number - + // SHN_ABS (0xFFF1) in particular is common and means the value is already final. + // Indexing sectionAddrs (which has GetNumSections() entries) with one of those read + // far out of bounds and added whatever it found to the symbol's address. + if (sectionIndex == SHN_UNDEF || sectionIndex >= SHN_LORESERVE) { + // Undefined, absolute or common - nothing of ours to relocate against. + continue; + } + if (sectionIndex >= GetNumSections()) { + WARN_LOG(Log::Loader, "Symbol '%s' refers to bad section %d, skipping", name, sectionIndex); + continue; + } value += sectionAddrs[sectionIndex]; + } switch (type) { diff --git a/Core/ELF/ElfReader.h b/Core/ELF/ElfReader.h index 132a7f8669..4d3192e7fa 100644 --- a/Core/ELF/ElfReader.h +++ b/Core/ELF/ElfReader.h @@ -51,10 +51,14 @@ public: ElfReader(const void *ptr, size_t size) { base = (const char*)ptr; base32 = (const u32 *)ptr; + size_ = size; + // Don't read the header to find the segment and section tables before we know it's there. + // LoadInto() rejects anything this small; header stays null so nothing else can use it either. + if (size < sizeof(Elf32_Ehdr)) + return; header = (const Elf32_Ehdr*)ptr; segments = (const Elf32_Phdr *)(base + header->e_phoff); sections = (const Elf32_Shdr *)(base + header->e_shoff); - size_ = size; } ~ElfReader() { @@ -66,21 +70,22 @@ public: return base32[off >> 2]; } - // Quick accessors - ElfType GetType() const { return (ElfType)(u16)(header->e_type); } - ElfMachine GetMachine() const { return (ElfMachine)(u16)(header->e_machine); } + // Quick accessors. header is null if we weren't even handed a full ELF header, see the + // constructor - so these all have to cope with that. + ElfType GetType() const { return header ? (ElfType)(u16)(header->e_type) : (ElfType)0; } + ElfMachine GetMachine() const { return header ? (ElfMachine)(u16)(header->e_machine) : (ElfMachine)0; } u32 GetEntryPoint() const { return entryPoint; } - u32 GetFlags() const { return (u32)(header->e_flags); } + u32 GetFlags() const { return header ? (u32)(header->e_flags) : 0; } - int GetNumSegments() const { return (int)(header->e_phnum); } - int GetNumSections() const { return (int)(header->e_shnum); } + int GetNumSegments() const { return header ? (int)(header->e_phnum) : 0; } + int GetNumSections() const { return header ? (int)(header->e_shnum) : 0; } const char *GetSectionName(int section) const; const u8 *GetPtr(u32 offset) const { return (const u8*)base + offset; } // Note: zero is not a valid output, means unavailable. u32 GetSectionDataOffset(int section) const { - if (section < 0 || section >= header->e_shnum) + if (section < 0 || section >= GetNumSections()) return 0; if (sections[section].sh_type == SHT_NOBITS) return 0; @@ -88,19 +93,26 @@ public: } const u8 *GetSectionDataPtr(int section) const { u32 offset = GetSectionDataOffset(section); - if (offset == 0 || offset > size_) + // Note >=: an offset exactly at the end of the file addresses no bytes at all. + if (offset == 0 || offset >= size_) return nullptr; return GetPtr(offset); } const u8 *GetSegmentPtr(int segment) const { - if (segments[segment].p_offset > size_) + if (segment < 0 || segment >= GetNumSegments()) + return nullptr; + if (segments[segment].p_offset >= size_) return nullptr; return GetPtr(segments[segment].p_offset); } u32 GetSectionAddr(SectionID section) const { + if (section < 0 || section >= GetNumSections() || !sectionAddrs) + return 0; return sectionAddrs[section]; } int GetSectionSize(SectionID section) const { + if (section < 0 || section >= GetNumSections()) + return 0; return sections[section].sh_size; } diff --git a/Core/ELF/PBPReader.cpp b/Core/ELF/PBPReader.cpp index baca705045..96f9819352 100644 --- a/Core/ELF/PBPReader.cpp +++ b/Core/ELF/PBPReader.cpp @@ -29,16 +29,20 @@ PBPReader::PBPReader(FileLoader *fileLoader) { } fileSize_ = (size_t)fileLoader->FileSize(); - if (fileLoader->ReadAt(0, sizeof(header_), (u8 *)&header_) != sizeof(header_)) { + if (fileLoader->ReadAt(0, sizeof(header_), &header_) != sizeof(header_)) { ERROR_LOG(Log::Loader, "PBP is too small to be valid: %s", fileLoader->GetPath().c_str()); return; } if (memcmp(header_.magic, "\0PBP", 4) != 0) { - if (memcmp(header_.magic, "\nFLE", 4) != 0) { + // Split string so the \x7f escape doesn't swallow the E. This used to compare against + // "\nFLE", which is neither ELF's magic nor anything else - so every file that wasn't a PBP + // was reported as an ELF, and the error branch below was unreachable. + if (memcmp(header_.magic, "\x7f" "ELF", 4) == 0) { VERBOSE_LOG(Log::Loader, "%s: File actually an ELF, not a PBP", fileLoader->GetPath().c_str()); isELF_ = true; } else { - ERROR_LOG(Log::Loader, "Magic number in %s indicated no PBP: %s", fileLoader->GetPath().c_str(), header_.magic); + ERROR_LOG(Log::Loader, "Magic number in %s indicates neither PBP nor ELF: %02x %02x %02x %02x", + fileLoader->GetPath().c_str(), (u8)header_.magic[0], (u8)header_.magic[1], (u8)header_.magic[2], (u8)header_.magic[3]); } return; } @@ -63,7 +67,9 @@ bool PBPReader::GetSubFile(PBPSubFile file, std::vector *out) const { const u32 off = header_.offsets[(int)file]; out->resize(expected); - size_t bytes = file_->ReadAt(off, expected, &(*out)[0]); + if (expected == 0) + return true; + size_t bytes = file_->ReadAt(off, expected, out->data()); if (bytes != expected) { ERROR_LOG(Log::Loader, "PBP file read truncated: %d -> %d", (int)expected, (int)bytes); if (bytes < expected) { diff --git a/Core/ELF/PBPReader.h b/Core/ELF/PBPReader.h index 572f215ad5..8781aa66a7 100644 --- a/Core/ELF/PBPReader.h +++ b/Core/ELF/PBPReader.h @@ -20,6 +20,7 @@ #include +#include "Common/Common.h" #include "Common/CommonTypes.h" #include "Common/Swap.h" @@ -54,17 +55,25 @@ public: bool GetSubFileAsString(PBPSubFile file, std::string *out) const; size_t GetSubFileSize(PBPSubFile file) const { - int num = (int)file; - if (num < 7) { - return header_.offsets[file + 1] - header_.offsets[file]; - } else { - return fileSize_ - header_.offsets[file]; - } + const int num = (int)file; + if (num < 0 || num >= (int)ARRAY_SIZE(header_.offsets)) + return 0; + const u32 start = header_.offsets[num]; + // The last subfile runs to the end of the file, the rest to where the next one starts. + const u32 stop = num + 1 < (int)ARRAY_SIZE(header_.offsets) ? header_.offsets[num + 1] : (u32)fileSize_; + // These offsets come out of the file, so they aren't necessarily ordered or even inside it. + // Subtracting them blind produced a huge size from an underflow. + if (stop < start || stop > fileSize_) + return 0; + return stop - start; } private: FileLoader *file_ = nullptr; size_t fileSize_ = 0; - const PBPHeader header_{}; + // Not const: the constructor reads the file straight into this. It used to be, and was written + // through a cast that stripped the const away - which compiles, but lets the compiler assume the + // value never changes from the {} it was initialized with. + PBPHeader header_{}; bool isELF_ = false; }; diff --git a/Core/ELF/ParamSFO.cpp b/Core/ELF/ParamSFO.cpp index 0e605c6be5..f969547c02 100644 --- a/Core/ELF/ParamSFO.cpp +++ b/Core/ELF/ParamSFO.cpp @@ -15,6 +15,7 @@ // Official git repository and contact information can be found at // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. +#include #include #include @@ -227,14 +228,18 @@ int ParamSFOData::GetDataOffset(const u8 *paramsfo, size_t size, const char *dat const IndexTable *indexTables = (const IndexTable *)(paramsfo + sizeof(Header)); const u8 *key_start = paramsfo + header->key_table_start; - int data_start = header->data_table_start; + const size_t data_start = header->data_table_start; for (u32 i = 0; i < header->index_table_entries; i++) { + // In size_t throughout - these are u32s from the file, and mixing them with int meant the + // bounds check below was done in whatever type the promotion landed on. ReadSFO does the + // same arithmetic this way. size_t key_offset = header->key_table_start + indexTables[i].key_table_offset; if (key_offset >= size) continue; - if (data_start + indexTables[i].data_table_offset >= (int)size) + size_t data_offset = data_start + indexTables[i].data_table_offset; + if (data_offset >= size) continue; const char *key = (const char *)(key_start + indexTables[i].key_table_offset); @@ -243,13 +248,19 @@ int ParamSFOData::GetDataOffset(const u8 *paramsfo, size_t size, const char *dat continue; if (!strcmp(key, dataName)) { - return data_start + indexTables[i].data_table_offset; + return (int)data_offset; } } return -1; } +// How many bytes an entry gets in the data table. Everything written for it has to fit in here - +// the size loop below and the fill loop after it both go through this, so they can't disagree. +static u32 ReservedSize(const ParamSFOData::ValueData &value) { + return (u32)std::max(0, value.max_size); +} + void ParamSFOData::WriteSFO(u8 **paramsfo, size_t *size) const { size_t total_size = 0; size_t key_size = 0; @@ -266,7 +277,7 @@ void ParamSFOData::WriteSFO(u8 **paramsfo, size_t *size) const { for (const auto &[k, v] : values) { key_size += k.size() + 1; - data_size += v.max_size; + data_size += ReservedSize(v); header.index_table_entries++; } @@ -299,35 +310,49 @@ void ParamSFOData::WriteSFO(u8 **paramsfo, size_t *size) const { index_ptr->key_table_offset = offset; offset = (u16)(data_ptr - (data+header.data_table_start)); index_ptr->data_table_offset = offset; - index_ptr->param_max_len = v.max_size; + const u32 reserved = ReservedSize(v); + index_ptr->param_max_len = reserved; if (v.type == VT_INT) { index_ptr->param_fmt = 0x0404; - index_ptr->param_len = 4; + index_ptr->param_len = std::min(4u, reserved); - *(s32_le *)data_ptr = v.i_value; + if (reserved >= 4) + *(s32_le *)data_ptr = v.i_value; + else + WARN_LOG(Log::Loader, "SFO key '%s' is an int but only reserves %d bytes, dropping", k.c_str(), (int)reserved); } else if (v.type == VT_UTF8_SPE) { index_ptr->param_fmt = 0x0004; - index_ptr->param_len = (u32)v.u_value.size(); + // Raw data, no terminator, but it still has to fit in what the entry reserved. + const u32 len = std::min((u32)v.u_value.size(), reserved); + if (len != v.u_value.size()) + WARN_LOG(Log::Loader, "SFO key '%s': %d bytes of data truncated to %d", k.c_str(), (int)v.u_value.size(), (int)len); + index_ptr->param_len = len; - memset(data_ptr, 0, index_ptr->param_max_len); - memcpy(data_ptr, v.u_value.data(), index_ptr->param_len); + memset(data_ptr, 0, reserved); + memcpy(data_ptr, v.u_value.data(), len); } else if (v.type == VT_UTF8) { index_ptr->param_fmt = 0x0204; - index_ptr->param_len = (u32)v.s_value.size()+1; + // param_len counts the NUL terminator, so the string itself gets reserved - 1 bytes. + // Several callers pass the string's own length as max_size (see PSPLoaders.cpp), which + // used to overrun the entry by the terminator plus one more from the stray write below. + const u32 len = std::min((u32)v.s_value.size(), reserved ? reserved - 1 : 0); + if (len != v.s_value.size()) + WARN_LOG(Log::Loader, "SFO key '%s': string of %d chars truncated to %d", k.c_str(), (int)v.s_value.size(), (int)len); + index_ptr->param_len = reserved ? len + 1 : 0; - memcpy(data_ptr,v.s_value.c_str(),index_ptr->param_len); - data_ptr[index_ptr->param_len] = 0; + memset(data_ptr, 0, reserved); // Also supplies the terminator. + memcpy(data_ptr, v.s_value.data(), len); } memcpy(key_ptr,k.c_str(),k.size()); key_ptr[k.size()] = 0; - data_ptr += index_ptr->param_max_len; + data_ptr += reserved; key_ptr += k.size() + 1; index_ptr++; @@ -348,14 +373,18 @@ std::string ParamSFOData::GenerateFakeID(const Path &filename) const { std::string file = path.GetFilename(); + // Deliberately byte-wise and ASCII-only. Filenames are UTF-8, and a plain char is signed on x86 + // and unsigned on ARM - so summing chars directly gave Windows and Android different IDs for the + // same non-ASCII folder name, and toupper() on a negative value trips MSVC's debug CRT. ASCII + // names, which is very nearly all of them, produce exactly the same ID as before either way. int sumOfAllLetters = 0; for (char &c : file) { - sumOfAllLetters += c; + sumOfAllLetters += (unsigned char)c; // Get rid of some garbage characters than can arise when opening content URIs. Well, I've only seen '%', but... - if (strchr("%() []", c) != nullptr) { + if (c && strchr("%() []", c) != nullptr) { c = 'X'; - } else { - c = toupper(c); + } else if (c >= 'a' && c <= 'z') { + c = c - 'a' + 'A'; } } diff --git a/Core/ELF/PrxDecrypter.cpp b/Core/ELF/PrxDecrypter.cpp index edccbb2597..27ae004e44 100644 --- a/Core/ELF/PrxDecrypter.cpp +++ b/Core/ELF/PrxDecrypter.cpp @@ -1017,6 +1017,16 @@ static int pspDecryptType6(KirkState *kirk, const u8 *inbuf, u8 *outbuf, u32 siz int pspDecryptPRX(const u8 *inbuf, u8 *outbuf, u32 size, const u8 *seed) { + // Every type below reads the tag at 0xD0, the size at 0xB0 and key data as far as 0x150, and + // writes a KIRK header into outbuf at a fixed offset derived from sizeof(PSP_Header). Without + // this, a PRX declaring a tiny psp_size got read past its end, had a header written past the end + // of the (equally tiny) output buffer, and passed "size - offset" to KIRK as an unsigned + // underflow. Callers must supply at least a whole header on both sides. + if (size < sizeof(PSP_Header)) { + ERROR_LOG(Log::Loader, "PRX too small to decrypt: %d bytes, need at least %d", (int)size, (int)sizeof(PSP_Header)); + return -1; + } + KirkState kirk{}; kirk_init(&kirk); diff --git a/Core/HLE/sceKernelMemory.cpp b/Core/HLE/sceKernelMemory.cpp index 79948fc32a..3ab7d97093 100644 --- a/Core/HLE/sceKernelMemory.cpp +++ b/Core/HLE/sceKernelMemory.cpp @@ -1663,7 +1663,7 @@ static u32 sceKernelGetMemoryBlockAddr(u32 uid, u32 addr) { PartitionMemoryBlock *block = kernelObjects.Get(uid, error); if (block) { Memory::WriteOrException_U32(block->address, addr); - return hleLogInfo(Log::sceKernel, 0, "block address: %08x", block->address); + return hleLogDebug(Log::sceKernel, 0, "block address: %08x", block->address); } else { return hleLogError(Log::sceKernel, 0, "failed"); } diff --git a/Core/HLE/sceKernelModule.cpp b/Core/HLE/sceKernelModule.cpp index da425070ea..72711b87f7 100644 --- a/Core/HLE/sceKernelModule.cpp +++ b/Core/HLE/sceKernelModule.cpp @@ -1095,6 +1095,15 @@ enum : u32 { // filename is only used for dumping/metadata. static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 loadAddress, bool fromTop, std::string *error_string, u32 *magic, std::string_view filename, u32 &error) { + // The magic reads below need four bytes, and the ~SCE branch another four after that. Everything + // downstream checks its own sizes; this is just so we can look at the magic at all. The PBP path + // in __KernelLoadModule computes elfSize from two offsets in the file and doesn't floor it. + if (elfSize < 2 * sizeof(u32)) { + *error_string = "ELF file truncated - can't load"; + error = SCE_KERNEL_ERROR_FILEERR; + return nullptr; + } + PSPModule *module = new PSPModule(); kernelObjects.Create(module); loadedModules.insert(module->GetUID()); @@ -1159,16 +1168,23 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load elfSize = maxElfSize; ptr = newptr; int decryptedSize = pspDecryptPRX(in, (u8*)ptr, head->psp_size); - _dbg_assert_(decryptedSize <= (int)maxElfSize); - if (decryptedSize <= 0 && Read32(ptr + 0x150) == ELF_MAGIC) { + // If decryption got us nowhere, the PRX may simply not be encrypted - in which case the ELF + // starts right after the header. Check the source buffer, not the destination: on the paths + // where decryption bails early nothing has been written to newptr yet, so this used to read + // uninitialized heap to decide. psp_size is known to be <= the data we actually have. + if (decryptedSize <= 0 && head->psp_size >= 0x150 + sizeof(u32) && Read32(in + 0x150) == ELF_MAGIC) { decryptedSize = head->psp_size - 0x150; memcpy(newptr, in + 0x150, decryptedSize); // In this case it's definitely not compressed. Added assert below. } - // Don't accept ELFs over 24MB - nor ones with negative size, of course. - if (decryptedSize < 0 || decryptedSize > 24 * 1024 * 1024) { + // Don't accept ELFs over 24MB, ones bigger than the buffer we allocated for them - nor ones + // with negative size, of course. + if (decryptedSize < 0 || decryptedSize > 24 * 1024 * 1024 || decryptedSize > (int)maxElfSize) { *error_string = StringFromFormat("ELF/PRX corrupt, unreasonable decrypted size: %d", (u32)decryptedSize); + delete [] newptr; + module->Cleanup(); + kernelObjects.Destroy(module->GetUID()); // TODO: Might be the wrong error code. error = SCE_KERNEL_ERROR_FILEERR; return nullptr; @@ -1190,6 +1206,9 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load // Bail out cleanly here rather than falling through to parse whatever's left // in the buffer (still compressed, not a valid ELF) as if it were real code. *error_string = StringFromFormat("Module '%s' decompression failed", head->modname); + delete [] newptr; + module->Cleanup(); + kernelObjects.Destroy(module->GetUID()); // TODO: Might be the wrong error code. error = SCE_KERNEL_ERROR_FILEERR; return nullptr; @@ -1210,6 +1229,10 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load // This should happen for all "kernel" modules. *error_string = "Missing key"; delete [] newptr; + // ptr still points into this buffer, but nothing below reads it - and the exits further + // down all free newptr, so it has to be null by the time they're reached. + newptr = nullptr; + ptr = nullptr; module->isFake = true; strncpy(module->nm.name, head->modname, ARRAY_SIZE(module->nm.name)); module->nm.entry_addr = -1;