[Asterisk-code-review] res stasis recording: Allow symbolic links in configured rec... (asterisk[13])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/7956 )

Change subject: res_stasis_recording: Allow symbolic links in configured recordings dir.
......................................................................

res_stasis_recording: Allow symbolic links in configured recordings dir.

If any component of ast_config_AST_RECORDING_DIR is a symbolic link we
would incorrectly assume the ARI user was trying to escape the recording
path.  Create additional check to check the recording directory's
realpath, only deny access if both do not match.

This is needed by the testsuite when run by 'run-local'.

Change-Id: I9145e841865edadcb5f75cead3471ad06bbb56c0
---
M res/stasis_recording/stored.c
1 file changed, 18 insertions(+), 7 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Matthew Fredrickson: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/res/stasis_recording/stored.c b/res/stasis_recording/stored.c
index 3ad7d82..f916e68 100644
--- a/res/stasis_recording/stored.c
+++ b/res/stasis_recording/stored.c
@@ -314,6 +314,7 @@
 	RAII_VAR(char *, file_with_ext, NULL, ast_free);
 	int res;
 	struct stat file_stat;
+	int prefix_len = strlen(ast_config_AST_RECORDING_DIR);
 
 	errno = 0;
 
@@ -334,18 +335,28 @@
 	ast_string_field_build(recording, file, "%s/%s", dir, file);
 
 	if (!ast_begins_with(dir, ast_config_AST_RECORDING_DIR)) {
-		/* Attempt to escape the recording directory */
-		ast_log(LOG_WARNING, "Attempt to access invalid recording %s\n",
-			name);
-		errno = EACCES;
-		return NULL;
+		/* It's possible that one or more component of the recording path is
+		 * a symbolic link, this would prevent dir from ever matching. */
+		char *real_basedir = realpath(ast_config_AST_RECORDING_DIR, NULL);
+
+		if (!real_basedir || !ast_begins_with(dir, real_basedir)) {
+			/* Attempt to escape the recording directory */
+			ast_log(LOG_WARNING, "Attempt to access invalid recording directory %s\n",
+				dir);
+			ast_std_free(real_basedir);
+			errno = EACCES;
+
+			return NULL;
+		}
+
+		prefix_len = strlen(real_basedir);
+		ast_std_free(real_basedir);
 	}
 
 	/* The actual name of the recording is file with the config dir
 	 * prefix removed.
 	 */
-	ast_string_field_set(recording, name,
-		recording->file + strlen(ast_config_AST_RECORDING_DIR) + 1);
+	ast_string_field_set(recording, name, recording->file + prefix_len + 1);
 
 	file_with_ext = find_recording(dir, file);
 	if (!file_with_ext) {

-- 
To view, visit https://gerrit.asterisk.org/7956
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: I9145e841865edadcb5f75cead3471ad06bbb56c0
Gerrit-Change-Number: 7956
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180116/a8e1f67d/attachment-0001.html>