Merge pull request #14985 from hrydgard/crash-fixes

Try to fix some crashes from Android crash reporting

Common/Serialize/Serializer.cpp

@@ -106,10 +106,19 @@ void PointerWrap::DoVoid(void *data, int size) {
 	(*ptr) += size;
 }
 
+// Not exactly sane but might catch some corrupt files.
+const int MAX_SANE_STRING_LENGTH = 1024 * 1024;
+
 void Do(PointerWrap &p, std::string &x) {
 	int stringLen = (int)x.length() + 1;
 	Do(p, stringLen);
 
+	if (stringLen < 0 || stringLen > MAX_SANE_STRING_LENGTH) {
+		WARN_LOG(SAVESTATE, "Savestate failure: bad stringLen %d", stringLen);
+		p.SetError(PointerWrap::ERROR_FAILURE);
+		return;
+	}
+
 	switch (p.mode) {
 	case PointerWrap::MODE_READ: x = (char*)*p.ptr; break;
 	case PointerWrap::MODE_WRITE: memcpy(*p.ptr, x.c_str(), stringLen); break;
@@ -123,6 +132,12 @@ void Do(PointerWrap &p, std::wstring &x) {
 	int stringLen = sizeof(wchar_t) * ((int)x.length() + 1);
 	Do(p, stringLen);
 
+	if (stringLen < 0 || stringLen > MAX_SANE_STRING_LENGTH) {
+		WARN_LOG(SAVESTATE, "Savestate failure: bad stringLen %d", stringLen);
+		p.SetError(PointerWrap::ERROR_FAILURE);
+		return;
+	}
+
 	auto read = [&]() {
 		std::wstring r;
 		// In case unaligned, use memcpy.
@@ -144,6 +159,12 @@ void Do(PointerWrap &p, std::u16string &x) {
 	int stringLen = sizeof(char16_t) * ((int)x.length() + 1);
 	Do(p, stringLen);
 
+	if (stringLen < 0 || stringLen > MAX_SANE_STRING_LENGTH) {
+		WARN_LOG(SAVESTATE, "Savestate failure: bad stringLen %d", stringLen);
+		p.SetError(PointerWrap::ERROR_FAILURE);
+		return;
+	}
+
 	auto read = [&]() {
 		std::u16string r;
 		// In case unaligned, use memcpy.

Core/Config.cpp

@@ -1794,8 +1794,13 @@ bool Config::loadGameConfig(const std::string &pGameId, const std::string &title
 
 	auto postShaderSetting = iniFile.GetOrCreateSection("PostShaderSetting")->ToMap();
 	mPostShaderSetting.clear();
-	for (auto it : postShaderSetting) {
-		mPostShaderSetting[it.first] = std::stof(it.second);
+	for (const auto &it : postShaderSetting) {
+		float value = 0.0f;
+		if (sscanf(it.second.c_str(), "%f", &value)) {
+			mPostShaderSetting[it.first] = value;
+		} else {
+			WARN_LOG(LOADER, "Invalid float value string for param %s: '%s'", it.first.c_str(), it.second.c_str());
+		}
 	}
 
 	auto postShaderChain = iniFile.GetOrCreateSection("PostShaderList")->ToMap();

Core/ELF/ParamSFO.cpp

@@ -103,19 +103,22 @@ bool ParamSFOData::ReadSFO(const u8 *paramsfo, size_t size) {
 
 	const IndexTable *indexTables = (const IndexTable *)(paramsfo + sizeof(Header));
 
+	if (header->key_table_start > size || header->data_table_start > size) {
+		return false;
+	}
+
 	const u8 *key_start = paramsfo + header->key_table_start;
 	const u8 *data_start = paramsfo + header->data_table_start;
 
 	for (u32 i = 0; i < header->index_table_entries; i++)
 	{
 		const char *key = (const char *)(key_start + indexTables[i].key_table_offset);
-
 		switch (indexTables[i].param_fmt) {
 		case 0x0404:
 			{
 				// Unsigned int
 				const u32_le *data = (const u32_le *)(data_start + indexTables[i].data_table_offset);
-				SetValue(key,*data,indexTables[i].param_max_len);
+				SetValue(key, *data, indexTables[i].param_max_len);
 				VERBOSE_LOG(LOADER, "%s %08x", key, *data);
 			}
 			break;
@@ -132,7 +135,7 @@ bool ParamSFOData::ReadSFO(const u8 *paramsfo, size_t size) {
 			{
 				const char *utfdata = (const char *)(data_start + indexTables[i].data_table_offset);
 				VERBOSE_LOG(LOADER, "%s %s", key, utfdata);
-				SetValue(key,std::string(utfdata /*, indexTables[i].param_len*/), indexTables[i].param_max_len);
+				SetValue(key, std::string(utfdata /*, indexTables[i].param_len*/), indexTables[i].param_max_len);
 			}
 			break;
 		}

Core/ELF/ParamSFO.h

@@ -56,7 +56,11 @@ class ParamSFOData
 	bool WriteSFO(u8 **paramsfo, size_t *size);
 
 	bool ReadSFO(const std::vector<u8> &paramsfo) {
-		return ReadSFO(&paramsfo[0], paramsfo.size());
+		if (!paramsfo.empty()) {
+			return ReadSFO(&paramsfo[0], paramsfo.size());
+		} else {
+			return false;
+		}
 	}
 
 	int GetDataOffset(const u8 *paramsfo, std::string dataName);

Core/FileSystems/DirectoryFileSystem.cpp

@@ -311,11 +311,13 @@ size_t DirectoryFileHandle::Read(u8* pointer, s64 size)
 			size = needsTrunc_ - off;
 		}
 	}
+	if (size > 0) {
 #ifdef _WIN32
-	::ReadFile(hFile, (LPVOID)pointer, (DWORD)size, (LPDWORD)&bytesRead, 0);
+		::ReadFile(hFile, (LPVOID)pointer, (DWORD)size, (LPDWORD)&bytesRead, 0);
 #else
-	bytesRead = read(hFile, pointer, size);
+		bytesRead = read(hFile, pointer, size);
 #endif
+	}
 	return replay_ ? ReplayApplyDiskRead(pointer, (uint32_t)bytesRead, (uint32_t)size, inGameDir_, CoreTiming::GetGlobalTimeUs()) : bytesRead;
 }
 

Core/Util/GameManager.cpp

@@ -597,12 +597,12 @@ bool GameManager::InstallMemstickGame(struct zip *z, const Path &zipfile, const
 	zip_close(z);
 	z = nullptr;
 	installProgress_ = 1.0f;
-	installInProgress_ = false;
 	installError_ = "";
 	if (deleteAfter) {
 		File::Delete(zipfile);
 	}
 	InstallDone();
+	installInProgress_ = false;
 	return true;
 
 bail:
@@ -647,9 +647,9 @@ bool GameManager::InstallZippedISO(struct zip *z, int isoFileIndex, const Path &
 
 	z = 0;
 	installProgress_ = 1.0f;
-	installInProgress_ = false;
 	installError_ = "";
 	InstallDone();
+	installInProgress_ = false;
 	return true;
 }
 
@@ -670,9 +670,9 @@ bool GameManager::InstallRawISO(const Path &file, const std::string &originalNam
 		}
 	}
 	installProgress_ = 1.0f;
-	installInProgress_ = false;
 	installError_ = "";
 	InstallDone();
+	installInProgress_ = false;
 	return true;
 }
 

android/src/org/ppsspp/ppsspp/NativeActivity.java

@@ -1118,30 +1118,39 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) {
 			return;
 		}
 		if (requestCode == RESULT_LOAD_IMAGE) {
-			Uri selectedImage = data.getData();
-			if (selectedImage != null) {
-				if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
-					NativeApp.sendMessage("bgImage_updated", selectedImage.toString());
-				} else {
-					String[] filePathColumn = {MediaStore.Images.Media.DATA};
-					Cursor cursor = getContentResolver().query(selectedImage, filePathColumn, null, null, null);
-					if (cursor != null) {
-						cursor.moveToFirst();
-						int columnIndex = cursor.getColumnIndex(filePathColumn[0]);
-						String picturePath = cursor.getString(columnIndex);
-						cursor.close();
-
-						NativeApp.sendMessage("bgImage_updated", picturePath);
+			try {
+				Uri selectedImage = data.getData();
+				if (selectedImage != null) {
+					if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
+						NativeApp.sendMessage("bgImage_updated", selectedImage.toString());
+					} else {
+						String[] filePathColumn = {MediaStore.Images.Media.DATA};
+						Cursor cursor = getContentResolver().query(selectedImage, filePathColumn, null, null, null);
+						if (cursor != null) {
+							cursor.moveToFirst();
+							int columnIndex = cursor.getColumnIndex(filePathColumn[0]);
+							String picturePath = cursor.getString(columnIndex);
+							cursor.close();
+							NativeApp.sendMessage("bgImage_updated", picturePath);
+						}
 					}
 				}
+			} catch (Exception e) {
+				Log.w(TAG, "Exception receiving image: " + e.toString());
 			}
 		} else if (requestCode == RESULT_OPEN_DOCUMENT) {
 			Uri selectedFile = data.getData();
 			if (selectedFile != null) {
-				// Grab permanent permission so we can show it in recents list etc.
-				if (Build.VERSION.SDK_INT >= 19) {
-					getContentResolver().takePersistableUriPermission(selectedFile, Intent.FLAG_GRANT_READ_URI_PERMISSION);
+				try {
+					// Grab permanent permission so we can show it in recents list etc.
+					if (Build.VERSION.SDK_INT >= 19) {
+						getContentResolver().takePersistableUriPermission(selectedFile, Intent.FLAG_GRANT_READ_URI_PERMISSION);
+					}
+				} catch (Exception e) {
+					Log.w(TAG, "Exception getting permissions for document: " + e.toString());
 				}
+				// Even if we got an exception getting permissions, try to pass along the file. Maybe this version of Android
+				// doesn't need it.
 				Log.i(TAG, "Browse file finished:" + selectedFile.toString());
 				NativeApp.sendMessage("browse_fileSelect", selectedFile.toString());
 			}
@@ -1150,7 +1159,13 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) {
 			if (selectedDirectoryUri != null) {
 				String path = selectedDirectoryUri.toString();
 				Log.i(TAG, "Browse folder finished: " + path);
-				getContentResolver().takePersistableUriPermission(selectedDirectoryUri, Intent.FLAG_GRANT_READ_URI_PERMISSION | Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
+				try {
+					getContentResolver().takePersistableUriPermission(selectedDirectoryUri, Intent.FLAG_GRANT_READ_URI_PERMISSION | Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
+				} catch (Exception e) {
+					Log.w(TAG, "Exception getting permissions for document: " + e.toString());
+				}
+				// Even if we got an exception getting permissions, try to pass along the file. Maybe this version of Android
+				// doesn't need it. If we can't access it, we'll fail in some other way later.
 				DocumentFile documentFile = DocumentFile.fromTreeUri(this, selectedDirectoryUri);
 				Log.i(TAG, "Document name: " + documentFile.getUri());
 				/*