[Scummvm-git-logs] scummvm master -> 625c1d10c8afaead595f9702aaf614cd7c70a9f2

lephilousophe noreply at scummvm.org
Sun Feb 26 11:20:50 UTC 2023


This automated email contains information about 3 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
98a0aab3f4 BACKENDS: NETWORKING: Set pointer to nullptr when it's deleted
9d879cb04f ANDROID: Allow to create a /saf node from path
625c1d10c8 ANDROID: Allow SAF non-existent node creation from path


Commit: 98a0aab3f4a8122b59f4e3a9cf278bc2b2ec32b9
    https://github.com/scummvm/scummvm/commit/98a0aab3f4a8122b59f4e3a9cf278bc2b2ec32b9
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2023-02-26T12:20:13+01:00

Commit Message:
BACKENDS: NETWORKING: Set pointer to nullptr when it's deleted

This avoids a UAF when file failed to open.

Changed paths:
    backends/networking/curl/sessionrequest.cpp


diff --git a/backends/networking/curl/sessionrequest.cpp b/backends/networking/curl/sessionrequest.cpp
index a0247480e68..0d5f145e433 100644
--- a/backends/networking/curl/sessionrequest.cpp
+++ b/backends/networking/curl/sessionrequest.cpp
@@ -58,6 +58,7 @@ void SessionRequest::openLocalFile(Common::String localFile) {
 		ErrorResponse error(this, false, true, "SessionRequestFile: unable to open file to download into", -1);
 		finishError(error);
 		delete _localFile;
+		_localFile = nullptr;
 		return;
 	}
 


Commit: 9d879cb04f72b8e8eaf5fa6b1d836b7154f16992
    https://github.com/scummvm/scummvm/commit/9d879cb04f72b8e8eaf5fa6b1d836b7154f16992
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2023-02-26T12:20:13+01:00

Commit Message:
ANDROID: Allow to create a /saf node from path

This avoids errors when creating parent directories in DumpFile
This also allows the user to specify /saf path in browser to allow
browsing.

Changed paths:
    backends/fs/android/android-fs-factory.cpp
    backends/fs/android/android-posix-fs.cpp
    backends/fs/android/android-saf-fs.cpp
    backends/fs/android/android-saf-fs.h


diff --git a/backends/fs/android/android-fs-factory.cpp b/backends/fs/android/android-fs-factory.cpp
index e478b614548..af3dcab9416 100644
--- a/backends/fs/android/android-fs-factory.cpp
+++ b/backends/fs/android/android-fs-factory.cpp
@@ -53,10 +53,14 @@ AbstractFSNode *AndroidFilesystemFactory::makeFileNodePath(const Common::String
 		return makeRootFileNode();
 	}
 
-	// No need to take SAF add mode here as it's called only for paths and we won't accept /saf path to make a new SAF
-
 	// If SAF works, it was a SAF URL
 	if (_withSAF) {
+		// Accept /saf as it can be used to create the tree in DumpFile
+		if (path == AddSAFFakeNode::SAF_ADD_FAKE_PATH) {
+			// Not a SAF mount point
+			return new AddSAFFakeNode(true);
+		}
+
 		AbstractFSNode *node = AndroidSAFFilesystemNode::makeFromPath(path);
 		if (node) {
 			return node;
@@ -90,7 +94,7 @@ void AndroidFilesystemFactory::getSAFTrees(AbstractFSList &list, bool allowSAFad
 	}
 
 	if (allowSAFadd) {
-		list.push_back(new AddSAFFakeNode());
+		list.push_back(new AddSAFFakeNode(false));
 	}
 
 }
diff --git a/backends/fs/android/android-posix-fs.cpp b/backends/fs/android/android-posix-fs.cpp
index f607c767d2b..2bc7fd4e7a9 100644
--- a/backends/fs/android/android-posix-fs.cpp
+++ b/backends/fs/android/android-posix-fs.cpp
@@ -30,13 +30,7 @@ AbstractFSNode *AndroidPOSIXFilesystemNode::makeNode() const {
 }
 
 AbstractFSNode *AndroidPOSIXFilesystemNode::makeNode(const Common::String &path) const {
-	// If SAF works, it was a SAF URL
-	AbstractFSNode *node = AndroidSAFFilesystemNode::makeFromPath(path);
-	if (node) {
-		return node;
-	}
-
-	return new AndroidPOSIXFilesystemNode(path, _config);
+	return AndroidFilesystemFactory::instance().makeFileNodePath(path);
 }
 
 #endif
diff --git a/backends/fs/android/android-saf-fs.cpp b/backends/fs/android/android-saf-fs.cpp
index b905386b5b0..0d7a0b227ac 100644
--- a/backends/fs/android/android-saf-fs.cpp
+++ b/backends/fs/android/android-saf-fs.cpp
@@ -586,6 +586,10 @@ AddSAFFakeNode::~AddSAFFakeNode() {
 }
 
 AbstractFSNode *AddSAFFakeNode::getChild(const Common::String &name) const {
+	if (_fromPath) {
+		// When starting from /saf try to get the tree node
+		return AndroidSAFFilesystemNode::makeFromPath(Common::String(AndroidSAFFilesystemNode::SAF_MOUNT_POINT) + name);
+	}
 	// We can't call getChild as it's protected
 	return nullptr;
 }
@@ -596,6 +600,11 @@ AbstractFSNode *AddSAFFakeNode::getParent() const {
 }
 
 bool AddSAFFakeNode::exists() const {
+	if (_fromPath) {
+		// /saf always exists when created as a path
+		return true;
+	}
+
 	if (!_proxied) {
 		makeProxySAF();
 	}
@@ -608,6 +617,16 @@ bool AddSAFFakeNode::exists() const {
 }
 
 bool AddSAFFakeNode::getChildren(AbstractFSList &list, ListMode mode, bool hidden) const {
+	if (_fromPath) {
+		// When built from path, /saf lists all SAF node but never proposes to add one
+		if (mode == Common::FSNode::kListFilesOnly) {
+			// All directories
+			return true;
+		}
+		AndroidFilesystemFactory::instance().getSAFTrees(list, false);
+		return true;
+	}
+
 	if (!_proxied) {
 		makeProxySAF();
 	}
@@ -620,6 +639,10 @@ bool AddSAFFakeNode::getChildren(AbstractFSList &list, ListMode mode, bool hidde
 }
 
 Common::String AddSAFFakeNode::getPath() const {
+	if (_fromPath) {
+		return SAF_ADD_FAKE_PATH;
+	}
+
 	if (!_proxied) {
 		makeProxySAF();
 	}
@@ -632,6 +655,10 @@ Common::String AddSAFFakeNode::getPath() const {
 }
 
 bool AddSAFFakeNode::isReadable() const {
+	if (_fromPath) {
+		return true;
+	}
+
 	if (!_proxied) {
 		makeProxySAF();
 	}
@@ -644,6 +671,10 @@ bool AddSAFFakeNode::isReadable() const {
 }
 
 bool AddSAFFakeNode::isWritable() const {
+	if (_fromPath) {
+		return false;
+	}
+
 	if (!_proxied) {
 		makeProxySAF();
 	}
@@ -656,6 +687,8 @@ bool AddSAFFakeNode::isWritable() const {
 }
 
 void AddSAFFakeNode::makeProxySAF() const {
+	assert(!_fromPath);
+
 	if (_proxied) {
 		return;
 	}
diff --git a/backends/fs/android/android-saf-fs.h b/backends/fs/android/android-saf-fs.h
index f6ff2690515..931c9f6b282 100644
--- a/backends/fs/android/android-saf-fs.h
+++ b/backends/fs/android/android-saf-fs.h
@@ -162,7 +162,7 @@ protected:
 public:
 	static const char SAF_ADD_FAKE_PATH[];
 
-	AddSAFFakeNode() : _proxied(nullptr) { }
+	AddSAFFakeNode(bool fromPath) : _proxied(nullptr), _fromPath(fromPath) { }
 	~AddSAFFakeNode() override;
 
 	bool exists() const override;
@@ -187,6 +187,7 @@ public:
 private:
 	void makeProxySAF() const;
 
+	bool _fromPath;
 	mutable AbstractFSNode *_proxied;
 };
 #endif


Commit: 625c1d10c8afaead595f9702aaf614cd7c70a9f2
    https://github.com/scummvm/scummvm/commit/625c1d10c8afaead595f9702aaf614cd7c70a9f2
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2023-02-26T12:20:13+01:00

Commit Message:
ANDROID: Allow SAF non-existent node creation from path

This is used by DumpFile.
As in all other implementations, parent node is expected to exist.

Changed paths:
    backends/fs/android/android-saf-fs.cpp


diff --git a/backends/fs/android/android-saf-fs.cpp b/backends/fs/android/android-saf-fs.cpp
index 0d7a0b227ac..cfa2510794a 100644
--- a/backends/fs/android/android-saf-fs.cpp
+++ b/backends/fs/android/android-saf-fs.cpp
@@ -167,17 +167,64 @@ AndroidSAFFilesystemNode *AndroidSAFFilesystemNode::makeFromPath(const Common::S
 		return nullptr;
 	}
 
-	if (!node) {
+	if (node) {
+		AndroidSAFFilesystemNode *ret = new AndroidSAFFilesystemNode(safTree, node);
+
+		env->DeleteLocalRef(node);
+		env->DeleteLocalRef(safTree);
+
+		return ret;
+	}
+
+	// Node doesn't exist: we will try to make a node from the parent and
+	// if it works we will create a non-existent node
+
+	pos = realPath.findLastOf('/');
+	if (pos == Common::String::npos || pos == 0) {
+		// No / in path or at root, no parent and we have a tree: it's all good
+		if (pos == 0) {
+			realPath = realPath.substr(1);
+		}
+		AndroidSAFFilesystemNode *parent = makeFromTree(safTree);
+		AndroidSAFFilesystemNode *ret = static_cast<AndroidSAFFilesystemNode *>(parent->getChild(realPath));
+		delete parent;
+
+		// safTree has already been released by makeFromTree
+		return ret;
+	}
+
+	Common::String baseName(realPath.substr(pos + 1));
+	realPath.erase(pos);
+
+	pathObj = env->NewStringUTF(realPath.c_str());
+
+	node = env->CallObjectMethod(safTree, _MID_pathToNode, pathObj);
+
+	env->DeleteLocalRef(pathObj);
+
+	if (env->ExceptionCheck()) {
+		LOGE("SAFFSTree::pathToNode failed");
+
+		env->ExceptionDescribe();
+		env->ExceptionClear();
+
 		env->DeleteLocalRef(safTree);
 		return nullptr;
 	}
 
-	AndroidSAFFilesystemNode *ret = new AndroidSAFFilesystemNode(safTree, node);
+	if (node) {
+		AndroidSAFFilesystemNode *parent = new AndroidSAFFilesystemNode(safTree, node);
+		env->DeleteLocalRef(node);
+		env->DeleteLocalRef(safTree);
 
-	env->DeleteLocalRef(node);
-	env->DeleteLocalRef(safTree);
+		AndroidSAFFilesystemNode *ret = static_cast<AndroidSAFFilesystemNode *>(parent->getChild(baseName));
+		delete parent;
 
-	return ret;
+		return ret;
+	}
+
+	env->DeleteLocalRef(safTree);
+	return nullptr;
 }
 
 AndroidSAFFilesystemNode *AndroidSAFFilesystemNode::makeFromTree(jobject safTree) {




More information about the Scummvm-git-logs mailing list