Update chroot symlink check (#765)

* start sftp chroot symlink fix

* update symlink logic chroot check

* fix order

Author: tgauth

contrib/win32/win32compat/fileio.c

@@ -435,14 +435,20 @@ file_in_chroot_jail(HANDLE handle) {
 		return 1;
 	}
 
+	return file_in_chroot_jail_helper(final_path);
+}
+
+/* returns 1 if true, 0 otherwise */
+int
+file_in_chroot_jail_helper(wchar_t* final_path) {
+	/* ensure final path is within chroot */
 	to_wlower_case(final_path);
 	if ((wcslen(final_path) < wcslen(chroot_pathw)) ||
-	    memcmp(final_path, chroot_pathw, 2 * wcslen(chroot_pathw)) != 0 ||
-	    final_path[wcslen(chroot_pathw)] != '\\') {
+		memcmp(final_path, chroot_pathw, 2 * wcslen(chroot_pathw)) != 0 ||
+		final_path[wcslen(chroot_pathw)] != '\\') {
 		debug3("access denied due to attempt to escape chroot jail");
 		return 0;
 	}
-
 	return 1;
 }
 
@@ -1268,6 +1274,7 @@ fileio_symlink(const char *target, const char *linkpath)
 	DWORD ret = -1;
 	char target_modified[PATH_MAX] = { 0 };
 	char *linkpath_resolved = NULL, *target_resolved = NULL;
+	wchar_t *linkpath_utf16 = NULL, *resolved_target_utf16 = NULL, *resolved_target_chroot = NULL;
 
 	if (target == NULL || linkpath == NULL) {
 		errno = EFAULT;
@@ -1301,13 +1308,21 @@ fileio_symlink(const char *target, const char *linkpath)
 		strcpy_s(target_modified, _countof(target_modified), target_resolved);
 	}
 
-	wchar_t *linkpath_utf16 = resolved_path_utf16(linkpath);
-	wchar_t *resolved_target_utf16 = utf8_to_utf16(target_modified);
-	if (resolved_target_utf16 == NULL || linkpath_utf16 == NULL) {
+	if ((linkpath_utf16 = resolved_path_utf16(linkpath)) == NULL || 
+		(resolved_target_utf16 = utf8_to_utf16(target_modified)) == NULL) {
 		errno = ENOMEM;
 		goto cleanup;
 	}
 
+	/* if chroot, get full path for target, similar to behavior in realpath() in misc.c 
+	   note: _wfullpath() is required to resolve paths containing unicode characters */
+	if (chroot_pathw != NULL &&
+		(resolved_target_chroot = _wfullpath(NULL, resolved_target_utf16, 0)) != NULL &&
+		file_in_chroot_jail_helper(resolved_target_chroot) != 1) {
+		errno = EPERM;
+		goto cleanup;
+	}
+
 	/* unlike other platforms, we need to know whether the symbolic link target is
 	 * a file or a directory.  the only way we can confidently do this is to
 	 * get the attributes of the target.  therefore, our symlink() has the
@@ -1338,15 +1353,18 @@ fileio_symlink(const char *target, const char *linkpath)
 	ret = 0;
 cleanup:
 
+	if (linkpath_resolved)
+		free(linkpath_resolved);
+
 	if (linkpath_utf16)
 		free(linkpath_utf16);
 
+	if (resolved_target_chroot)
+		free(resolved_target_chroot);
+
 	if (resolved_target_utf16)
 		free(resolved_target_utf16);
 
-	if (linkpath_resolved)
-		free(linkpath_resolved);
-
 	if (target_resolved)
 		free(target_resolved);
 

contrib/win32/win32compat/misc_internal.h

@@ -70,6 +70,7 @@ int load_user_profile(HANDLE user_token, char* user);
 int create_directory_withsddl(wchar_t *path, wchar_t *sddl, BOOL check_permissions);
 int is_absolute_path(const char *);
 int file_in_chroot_jail(HANDLE);
+int file_in_chroot_jail_helper(wchar_t*);
 PSID lookup_sid(const wchar_t* name_utf16, PSID psid, DWORD * psid_len);
 PSID get_sid(const char*);
 int am_system();