[io] check that file path doesn't escape app's local data folder when opening files

samples/pong/build.sh

@@ -12,4 +12,4 @@ wasmFlags="--target=wasm32 \
 
 /usr/local/opt/llvm/bin/clang $wasmFlags -o ./module.wasm ../../sdk/orca.c src/main.c
 
-python3 ../../scripts/mkapp.py --orca-dir ../.. --name Pong --icon icon.png module.wasm
+python3 ../../scripts/mkapp.py --orca-dir ../.. --name Pong --icon icon.png --data-dir dir1 module.wasm

samples/pong/dir1/test_read.txt

@@ -0,0 +1 @@
+Hello, ressource file

samples/pong/src/main.c

@@ -41,13 +41,13 @@ void OnInit(void)
 	surface = mg_surface_main();
 	canvas = mg_canvas_create();
 
-	file_handle file = file_open(STR8("test_file.txt")	, IO_OPEN_CREATE | IO_OPEN_WRITE);
+	file_handle file = file_open(STR8("/test_write.txt")	, IO_OPEN_CREATE | IO_OPEN_WRITE);
 
 	str8 string = STR8("Hello, file!\n");
 	file_write(file, string.len, string.ptr);
 	file_close(file);
 
-	file = file_open(STR8("test_file.txt")	, IO_OPEN_READ);
+	file = file_open(STR8("/dir1/test_read.txt")	, IO_OPEN_READ);
 	u64 size = file_size(file);
 	char* buffer = mem_arena_alloc(mem_scratch(), size);
 	file_read(file, size, buffer);

scripts/mkapp.py

@@ -18,10 +18,10 @@ from argparse import ArgumentParser
 #----------------------------------------------------------------------------------------------
 
 parser = ArgumentParser(prog='mkapp')
-parser.add_argument("-r", "--res-file", nargs='+', action='append', dest='res_files')
+parser.add_argument("-d", "--data-file", action='append', dest='data_files')
-parser.add_argument("-R", "--res-dir", nargs='+', action='append', dest='res_dirs')
+parser.add_argument("-D", "--data-dir", action='append', dest='data_dirs')
 parser.add_argument("-i", "--icon")
-parser.add_argument("-D", "--out-dir", default=os.getcwd())
+parser.add_argument("-C", "--out-dir", default=os.getcwd())
 parser.add_argument("-n", "--name", default='out')
 parser.add_argument("-O", "--orca-dir", default='.')
 parser.add_argument("--version", default='0.0.0')
@@ -38,8 +38,9 @@ bundle_path = args.out_dir + '/' + bundle_name
 contents_dir = bundle_path + '/Contents'
 exe_dir = contents_dir + '/MacOS'
 res_dir = contents_dir + '/resources'
-wasm_dir = contents_dir + '/wasm'
+guest_dir = contents_dir + '/app'
-data_dir = contents_dir + '/data'
+wasm_dir = guest_dir + '/wasm'
+data_dir = guest_dir + '/data'
 
 if os.path.exists(bundle_path):
 	shutil.rmtree(bundle_path)
@@ -47,6 +48,7 @@ os.mkdir(bundle_path)
 os.mkdir(contents_dir)
 os.mkdir(exe_dir)
 os.mkdir(res_dir)
+os.mkdir(guest_dir)
 os.mkdir(wasm_dir)
 os.mkdir(data_dir)
 
@@ -66,23 +68,27 @@ shutil.copy(egl_lib, exe_dir)
 shutil.copy(renderer_lib, exe_dir)
 
 #-----------------------------------------------------------
-#NOTE: copy wasm module and resources
+#NOTE: copy wasm module and data
 #-----------------------------------------------------------
 shutil.copy(args.module, wasm_dir + '/module.wasm')
 
-if args.res_files != None:
+if args.data_files != None:
-	for res in args.res_files:
+	for data in args.data_files:
-		shutil.copy(res, res_dir)
+		shutil.copy(data, data_dir)
 
-if args.res_dirs != None:
+if args.data_dirs != None:
-	for res in args.res_dirs:
+	for data in args.data_dirs:
-		shutil.copytree(res, res_dir)
+		shutil.copytree(data, data_dir + '/' + os.path.basename(data), dirs_exist_ok=True)
 
+#-----------------------------------------------------------
+#NOTE: copy runtime resources
+#-----------------------------------------------------------
 # default fonts
 shutil.copy(args.orca_dir + '/resources/OpenSansLatinSubset.ttf', res_dir)
 shutil.copy(args.orca_dir + '/resources/Menlo.ttf', res_dir)
 shutil.copy(args.orca_dir + '/resources/Menlo Bold.ttf', res_dir)
 shutil.copy(args.orca_dir + '/resources/Menlo Italics.ttf', res_dir)
+
 #-----------------------------------------------------------
 #NOTE make icon
 #-----------------------------------------------------------

src/io_common.h

@@ -70,20 +70,27 @@ typedef i32 io_error;
 enum {
 	IO_OK = 0,
 	IO_ERR_UNKNOWN,
-	IO_ERR_OP,      // unsupported operation
+	IO_ERR_OP,          // unsupported operation
-	IO_ERR_HANDLE,  // invalid handle
+	IO_ERR_HANDLE,      // invalid handle
-	IO_ERR_PREV,    // previously had a fatal error (last error stored on handle)
+	IO_ERR_PREV,        // previously had a fatal error (last error stored on handle)
-	IO_ERR_ARG,     // invalid argument or argument combination
+	IO_ERR_ARG,         // invalid argument or argument combination
-	IO_ERR_PERM,    // access denied
+	IO_ERR_PERM,        // access denied
-	IO_ERR_SPACE,   // no space left
+	IO_ERR_SPACE,       // no space left
-	IO_ERR_NO_FILE, // file does not exist
+	IO_ERR_NO_ENTRY,    // file or directory does not exist
-	IO_ERR_EXISTS,  // file already exists
+	IO_ERR_EXISTS,      // file already exists
-	IO_ERR_MAX_FILES, // max open files reached
+	IO_ERR_NOT_DIR,     // path element is not a directory
+	IO_ERR_DIR,         // attempted to write directory
+	IO_ERR_MAX_FILES,   // max open files reached
+	IO_ERR_MAX_LINKS,   // too many symbolic links in path
 	IO_ERR_PATH_LENGTH, // path too long
 	IO_ERR_FILE_SIZE,   // file too big
 	IO_ERR_OVERFLOW,    // offset too big
-	IO_ERR_NOT_READY, // no data ready to be read/written
+	IO_ERR_NOT_READY,   // no data ready to be read/written
-	IO_ERR_MEM,       // failed to allocate memory
+	IO_ERR_MEM,         // failed to allocate memory
+	IO_ERR_INTERRUPT,   // operation interrupted by a signal
+	IO_ERR_PHYSICAL,    // physical IO error
+	IO_ERR_NO_DEVICE,   // device not found
+	IO_ERR_WALKOUT,     // attempted to walk out of root directory
 
 	//...
 };

src/io_impl.c

@@ -9,6 +9,7 @@
 #include<fcntl.h>
 #include<sys/stat.h>
 #include<unistd.h>
+#include<limits.h>
 #include"io_common.h"
 
 //TODO: - file_handle to FILE* association
@@ -84,6 +85,278 @@ file_slot* file_slot_from_handle(file_table* table, file_handle handle)
 	return(slot);
 }
 
+io_error io_convert_errno(int e)
+{
+	io_error error;
+	switch(e)
+	{
+		case EPERM:
+		case EACCES:
+		case EROFS:
+			error = IO_ERR_PERM;
+			break;
+
+		case ENOENT:
+			error = IO_ERR_NO_ENTRY;
+			break;
+
+		case EINTR:
+			error = IO_ERR_INTERRUPT;
+			break;
+
+		case EIO:
+			error = IO_ERR_PHYSICAL;
+			break;
+
+		case ENXIO:
+			error = IO_ERR_NO_DEVICE;
+			break;
+
+		case EBADF:
+			// this should only happen when user tries to write/read to a file handle
+			// opened with readonly/writeonly access
+			error = IO_ERR_PERM;
+			break;
+
+		case ENOMEM:
+			error = IO_ERR_MEM;
+			break;
+
+		case EFAULT:
+		case EINVAL:
+		case EDOM:
+			error = IO_ERR_ARG;
+			break;
+
+		case EBUSY:
+		case EAGAIN:
+			error = IO_ERR_NOT_READY;
+			break;
+
+		case EEXIST:
+			error = IO_ERR_EXISTS;
+			break;
+
+		case ENOTDIR:
+			error = IO_ERR_NOT_DIR;
+			break;
+
+		case EISDIR:
+			error = IO_ERR_DIR;
+			break;
+
+		case ENFILE:
+		case EMFILE:
+			error = IO_ERR_MAX_FILES;
+			break;
+
+		case EFBIG:
+			error = IO_ERR_FILE_SIZE;
+			break;
+
+		case ENOSPC:
+		case EDQUOT:
+			error = IO_ERR_SPACE;
+			break;
+
+		case ELOOP:
+			error = IO_ERR_MAX_LINKS;
+			break;
+
+		case ENAMETOOLONG:
+			error = IO_ERR_PATH_LENGTH;
+			break;
+
+		case EOVERFLOW:
+			error = IO_ERR_OVERFLOW;
+			break;
+
+		default:
+			error = IO_ERR_UNKNOWN;
+			break;
+	}
+	return(error);
+}
+
+typedef struct io_path_walk_result
+{
+	io_error error;
+	int fd;
+
+} io_path_walk_result;
+
+io_path_walk_result io_path_walk(str8 path)
+{
+	io_path_walk_result result = {.fd = -1};
+
+	mem_arena* scratch = mem_scratch();
+	mem_arena_marker mark = mem_arena_mark(scratch);
+
+	str8_list sep = {0};
+	str8_list_push(scratch, &sep, STR8("/"));
+	str8_list pathElements = str8_split(scratch, path, sep);
+
+	str8 execPath = mp_path_directory(mp_app_get_executable_path(scratch));
+	str8_list list = {0};
+	str8_list_push(scratch, &list, execPath);
+	str8_list_push(scratch, &list, STR8("../app/data"));
+	str8 dataPath = str8_list_join(scratch, list);
+
+	result.fd = open(str8_to_cstring(scratch, dataPath), O_RDONLY);
+	if(result.fd < 0)
+	{
+		result.error = io_convert_errno(errno);
+		goto error;
+	}
+	struct stat dirStat;
+	if(fstat(result.fd, &dirStat))
+	{
+		result.error = io_convert_errno(errno);
+		goto error;
+	}
+	ino_t baseInode = dirStat.st_ino;
+	ino_t currentInode = baseInode;
+
+	for_list(&pathElements.list, elt, str8_elt, listElt)
+	{
+		str8 name = elt->string;
+
+		if(!str8_cmp(name, STR8(".")))
+		{
+			//NOTE: skip
+			continue;
+		}
+		else if(!str8_cmp(name, STR8("..")))
+		{
+			//NOTE: check that we don't escape 'root' dir
+			if(currentInode == baseInode)
+			{
+				result.error = IO_ERR_WALKOUT;
+				goto error;
+			}
+			else
+			{
+				// open that dir and continue
+				int nextFd = openat(result.fd, "..", O_RDONLY);
+				if(!nextFd)
+				{
+					result.error = io_convert_errno(errno);
+					goto error;
+				}
+				else
+				{
+					close(result.fd);
+					result.fd = nextFd;
+					if(fstat(result.fd, &dirStat))
+					{
+						result.error = io_convert_errno(errno);
+						goto error;
+					}
+					currentInode = dirStat.st_ino;
+				}
+			}
+		}
+		else
+		{
+			char* nameCStr = str8_to_cstring(scratch, name);
+			struct stat st;
+
+			if(fstatat(result.fd, nameCStr, &st, AT_SYMLINK_NOFOLLOW))
+			{
+				if(&elt->listElt != list_last(&pathElements.list))
+				{
+					result.error = io_convert_errno(errno);
+					goto error;
+				}
+				else
+				{
+					continue;
+				}
+			}
+
+			int type = st.st_mode & S_IFMT;
+
+			if(type == S_IFLNK)
+			{
+				// symlink, check that it's relative, and insert it in elements
+				char buff[PATH_MAX+1];
+				int r = readlinkat(result.fd, nameCStr, buff, PATH_MAX);
+				if(r<0)
+				{
+					result.error = io_convert_errno(errno);
+					goto error;
+				}
+				else if(r == 0)
+				{
+					//NOTE: skip
+				}
+				else if(buff[0] == '/')
+				{
+					result.error = IO_ERR_WALKOUT;
+					goto error;
+				}
+				else
+				{
+					buff[r] = '\0';
+
+					str8_list linkElements = str8_split(scratch, str8_from_buffer(r, buff), sep);
+					if(!list_empty(&linkElements.list))
+					{
+						//NOTE: insert linkElements into pathElements after elt
+						list_elt* tmp = elt->listElt.next;
+						elt->listElt.next = linkElements.list.first;
+						linkElements.list.last->next = tmp;
+						if(!tmp)
+						{
+							pathElements.list.last = linkElements.list.last;
+						}
+					}
+				}
+			}
+			else if(type == S_IFDIR)
+			{
+				// dir, open it and continue
+				int nextFd = openat(result.fd, nameCStr, O_RDONLY);
+				if(!nextFd)
+				{
+					result.error = io_convert_errno(errno);
+					goto error;
+				}
+				else
+				{
+					close(result.fd);
+					result.fd = nextFd;
+					currentInode = st.st_ino;
+				}
+			}
+			else if(type == S_IFREG)
+			{
+				// regular file, check that we're at the last element
+				if(&elt->listElt != list_last(&pathElements.list))
+				{
+					result.error = IO_ERR_NOT_DIR;
+					goto error;
+				}
+			}
+			else
+			{
+				result.error = IO_ERR_NOT_DIR;
+				goto error;
+			}
+		}
+	}
+	goto end;
+
+	error:
+	{
+		close(result.fd);
+		result.fd = -1;
+	}
+	end:
+	mem_arena_clear_to(scratch, mark);
+	return(result);
+}
+
 io_cmp io_open(io_req* req)
 {
 	io_cmp cmp = {0};
@@ -129,6 +402,8 @@ io_cmp io_open(io_req* req)
 			flags |= O_CREAT;
 		}
 
+		flags |= O_NOFOLLOW;
+
 		mode_t mode = S_IRUSR
 	            	| S_IWUSR
 	            	| S_IRGRP
@@ -136,88 +411,43 @@ io_cmp io_open(io_req* req)
 	            	| S_IROTH
 	            	| S_IWOTH;
 
-		//NOTE: build path
+		//NOTE: walk path (ensuring it's inside app's data directory subtree)
-		////////////////////////////////////////////////////////////////////////////////////
+		str8 path = str8_from_buffer(req->size, req->buffer);
-		//TODO: canonicalize directory path & check that it's inside local app folder
+		io_path_walk_result walkRes = io_path_walk(path);
-		////////////////////////////////////////////////////////////////////////////////////
+		if(walkRes.error != IO_OK)
-
-		mem_arena* scratch = mem_scratch();
-		mem_arena_marker mark = mem_arena_mark(scratch);
-
-		str8_list list = {0};
-
-		str8 execPath = mp_app_get_executable_path(scratch);
-		str8 execDir = mp_path_directory(execPath);
-
-		str8_list_push(scratch, &list, execDir);
-		str8_list_push(scratch, &list, STR8("/../data/"));
-		str8_list_push(scratch, &list, str8_from_buffer(req->size, req->buffer));
-
-		str8 absPath = str8_list_join(scratch, list);
-		char* absCString = str8_to_cstring(scratch, absPath);
-
-		//NOTE: open
-		int fd = open(absCString, flags, mode);
-
-		if(fd >= 0)
-		{
-			slot->fd = fd;
-			file_handle handle = file_handle_from_slot(&__globalFileTable, slot);
-			cmp.result = handle.h;
-		}
-		else
 		{
 			slot->fd = -1;
 			slot->fatal = true;
+			slot->error = walkRes.error;
+		}
+		else
+		{
+			//NOTE: open
+			mem_arena* scratch = mem_scratch();
+			mem_arena_marker mark = mem_arena_mark(scratch);
 
-			switch(errno)
+			str8 name = mp_path_base_name(path);
+			char* nameCStr = str8_to_cstring(scratch, name);
+			int fd = openat(walkRes.fd, nameCStr, flags, mode);
+			close(walkRes.fd);
+
+			mem_arena_clear_to(scratch, mark);
+
+			if(fd >= 0)
 			{
-				case EACCES:
+				slot->fd = fd;
-				case EROFS:
+				file_handle handle = file_handle_from_slot(&__globalFileTable, slot);
-					slot->error = IO_ERR_PERM;
+				cmp.result = handle.h;
-					break;
+			}
-
+			else
-				case EDQUOT:
+			{
-				case ENOSPC:
+				slot->fd = -1;
-					slot->error = IO_ERR_SPACE;
+				slot->fatal = true;
-					break;
+				slot->error = io_convert_errno(errno);
-
-				case EEXIST:
-					slot->error = IO_ERR_EXISTS;
-					break;
-
-				case EFAULT:
-				case EINVAL:
-				case EISDIR:
-					slot->error = IO_ERR_ARG;
-					break;
-
-				case EMFILE:
-				case ENFILE:
-					slot->error = IO_ERR_MAX_FILES;
-					break;
-
-				case ENAMETOOLONG:
-					slot->error = IO_ERR_PATH_LENGTH;
-					break;
-
-				case ENOENT:
-				case ENOTDIR:
-					slot->error = IO_ERR_NO_FILE;
-					break;
-
-				case EOVERFLOW:
-					slot->error = IO_ERR_FILE_SIZE;
-					break;
-
-				default:
-					slot->error = IO_ERR_UNKNOWN;
-					break;
 			}
-			cmp.error = slot->error;
 		}
 
-		mem_arena_clear_to(scratch, mark);
+		cmp.error = slot->error;
 	}
 
 	return(cmp);

src/main.c

@@ -363,7 +363,7 @@ void* orca_runloop(void* user)
 
 	//NOTE: loads wasm module
 	const char* bundleNameCString = "module";
-	str8 modulePath = mp_app_get_resource_path(mem_scratch(), "../wasm/module.wasm");
+	str8 modulePath = mp_app_get_resource_path(mem_scratch(), "../app/wasm/module.wasm");
 	const char* modulePathCString = str8_to_cstring(mem_scratch(), modulePath);
 
 	FILE* file = fopen(modulePathCString, "rb");