playlist_edit: move UID check to client_allow_file()

5 files changed, 129 insertions, 39 deletions

diff --git a/src/client_file.c b/src/client_file.c
new file mode 100644
index 000000000..e2d315701
--- /dev/null
+++ b/src/client_file.c
@@ -0,0 +1,65 @@
+/*
+ * Copyright (C) 2003-2012 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "client_file.h"
+#include "client.h"
+#include "ack.h"
+
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <errno.h>
+#include <unistd.h>
+
+bool
+client_allow_file(const struct client *client, const char *path_fs,
+		  GError **error_r)
+{
+#ifdef WIN32
+	(void)client;
+	(void)path_fs;
+
+	g_set_error(error_r, ack_quark(), ACK_ERROR_PERMISSION,
+		    "Access denied");
+	return false;
+#else
+	const int uid = client_get_uid(client);
+	if (uid <= 0) {
+		/* unauthenticated client */
+		g_set_error(error_r, ack_quark(), ACK_ERROR_PERMISSION,
+			    "Access denied");
+		return false;
+	}
+
+	struct stat st;
+	if (stat(path_fs, &st) < 0) {
+		g_set_error(error_r, g_file_error_quark(), errno,
+			    "%s", g_strerror(errno));
+		return false;
+	}
+
+	if (st.st_uid != (uid_t)uid && (st.st_mode & 0444) != 0444) {
+		/* client is not owner */
+		g_set_error(error_r, ack_quark(), ACK_ERROR_PERMISSION,
+			    "Access denied");
+		return false;
+	}
+
+	return true;
+#endif
+}
diff --git a/src/client_file.h b/src/client_file.h
new file mode 100644
index 000000000..3dcbe7500
--- /dev/null
+++ b/src/client_file.h
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2003-2012 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef MPD_CLIENT_FILE_H
+#define MPD_CLIENT_FILE_H
+
+#include <glib.h>
+#include <stdbool.h>
+
+struct client;
+
+/**
+ * Is this client allowed to use the specified local file?
+ *
+ * Note that this function is vulnerable to timing/symlink attacks.
+ * We cannot fix this as long as there are plugins that open a file by
+ * its name, and not by file descriptor / callbacks.
+ *
+ * @param path_fs the absolute path name in filesystem encoding
+ * @return true if access is allowed
+ */
+G_GNUC_PURE
+bool
+client_allow_file(const struct client *client, const char *path_fs,
+		  GError **error_r);
+
+#endif
diff --git a/src/command.c b/src/command.c
index 655ed7b1a..793737964 100644
--- a/src/command.c
+++ b/src/command.c
@@ -53,6 +53,7 @@
 #include "client_idle.h"
 #include "client_internal.h"
 #include "client_subscribe.h"
+#include "client_file.h"
 #include "tag_print.h"
 #include "path.h"
 #include "replay_gain_config.h"
@@ -441,14 +442,16 @@ handle_add(struct client *client, G_GNUC_UNUSED int argc, char *argv[])
 	enum playlist_result result;
 
 	if (strncmp(uri, "file:///", 8) == 0) {
-#ifdef WIN32
-		result = PLAYLIST_RESULT_DENIED;
-#else
+		const char *path = uri + 7;
+
+		GError *error = NULL;
+		if (!client_allow_file(client, path, &error))
+			return print_error(client, error);
+
 		result = playlist_append_file(&g_playlist,
 					      client->player_control,
-					      uri + 7, client_get_uid(client),
+					      path,
 					      NULL);
-#endif
 		return print_playlist_result(client, result);
 	}
 
@@ -479,15 +482,16 @@ handle_addid(struct client *client, int argc, char *argv[])
 	enum playlist_result result;
 
 	if (strncmp(uri, "file:///", 8) == 0) {
-#ifdef WIN32
-		result = PLAYLIST_RESULT_DENIED;
-#else
+		const char *path = uri + 7;
+
+		GError *error = NULL;
+		if (!client_allow_file(client, path, &error))
+			return print_error(client, error);
+
 		result = playlist_append_file(&g_playlist,
 					      client->player_control,
-					      uri + 7,
-					      client_get_uid(client),
+					      path,
 					      &added_id);
-#endif
 	} else {
 		if (uri_has_scheme(uri) && !uri_supported_scheme(uri)) {
 			command_error(client, ACK_ERROR_NO_EXIST,
diff --git a/src/playlist.h b/src/playlist.h
index caed0a220..a21bdf24a 100644
--- a/src/playlist.h
+++ b/src/playlist.h
@@ -100,15 +100,14 @@ playlist_get_queue(const struct playlist *playlist)
 void
 playlist_clear(struct playlist *playlist, struct player_control *pc);
 
-#ifndef WIN32
 /**
- * Appends a local file (outside the music database) to the playlist,
- * but only if the file's owner is equal to the specified uid.
+ * Appends a local file (outside the music database) to the playlist.
+ *
+ * Note: the caller is responsible for checking permissions.
  */
 enum playlist_result
 playlist_append_file(struct playlist *playlist, struct player_control *pc,
-		     const char *path, int uid, unsigned *added_id);
-#endif
+		     const char *path_fs, unsigned *added_id);
 
 enum playlist_result
 playlist_append_uri(struct playlist *playlist, struct player_control *pc,
diff --git a/src/playlist_edit.c b/src/playlist_edit.c
index dbd205a06..7adbccd7c 100644
--- a/src/playlist_edit.c
+++ b/src/playlist_edit.c
@@ -31,9 +31,6 @@
 #include "song.h"
 #include "idle.h"
 
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <unistd.h>
 #include <stdlib.h>
 
 static void playlist_increment_version(struct playlist *playlist)
@@ -63,34 +60,16 @@ playlist_clear(struct playlist *playlist, struct player_control *pc)
 	playlist_increment_version(playlist);
 }
 
-#ifndef WIN32
 enum playlist_result
 playlist_append_file(struct playlist *playlist, struct player_control *pc,
-		     const char *path, int uid, unsigned *added_id)
+		     const char *path_fs, unsigned *added_id)
 {
-	int ret;
-	struct stat st;
-	struct song *song;
-
-	if (uid <= 0)
-		/* unauthenticated client */
-		return PLAYLIST_RESULT_DENIED;
-
-	ret = stat(path, &st);
-	if (ret < 0)
-		return PLAYLIST_RESULT_ERRNO;
-
-	if (st.st_uid != (uid_t)uid && (st.st_mode & 0444) != 0444)
-		/* client is not owner */
-		return PLAYLIST_RESULT_DENIED;
-
-	song = song_file_load(path, NULL);
+	struct song *song = song_file_load(path_fs, NULL);
 	if (song == NULL)
 		return PLAYLIST_RESULT_NO_SUCH_SONG;
 
 	return playlist_append_song(playlist, pc, song, added_id);
 }
-#endif
 
 enum playlist_result
 playlist_append_song(struct playlist *playlist, struct player_control *pc,