From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751132AbdJCAu4 (ORCPT ); Mon, 2 Oct 2017 20:50:56 -0400 Received: from tartarus.angband.pl ([89.206.35.136]:59668 "EHLO tartarus.angband.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750846AbdJCAuy (ORCPT ); Mon, 2 Oct 2017 20:50:54 -0400 From: Adam Borowski To: Alexander Viro , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Adam Borowski Date: Tue, 3 Oct 2017 02:50:42 +0200 Message-Id: <20171003005042.16470-1-kilobyte@angband.pl> X-Mailer: git-send-email 2.14.2 X-SA-Exim-Connect-IP: 89.71.161.30 X-SA-Exim-Mail-From: kilobyte@angband.pl Subject: [PATCH] vfs: hard-ban creating files with control characters in the name X-SA-Exim-Version: 4.2.1 (built Tue, 02 Aug 2016 21:08:31 +0000) X-SA-Exim-Scanned: Yes (on tartarus.angband.pl) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Anything with bytes 1-31,127 will get -EACCES. Especially \n is bad: instead of natural file-per-line, you need an user-unfriendly feature of -print0 added to every producer and consumer; a good part of users either don't know or don't feel the need to bother with escaping this snowflake, thus introducing security holes. The rest of control characters, while not as harmful, don't have a legitimate use nor have any real chance of coming from a tarball (malice and fooling around excluded). No character set ever supported as a system locale by glibc, and, TTBMK, by other popular Unices, includes them, thus it can be assumed no foreign files have such names other than artificially. This goes in stark contrast with other characters proposed to be banned: non-UTF8 is common, and even on my desktop's disk I found examples of all of: [ ], < >, initial -, initial and final space, ?, *, .., ', ", |, &. Somehow no \ anywhere. I think I have an idea why no / . Another debatable point is whether to -EACCES or to silently rename to an escaped form such as %0A. I believe the former is better because: * programs can be confused if a directory has files they didn't just write * many filesystems already disallow certain characters (like invalid Unicode), thus returning an error is consistent An example of a write-up of this issue can be found at: https://www.dwheeler.com/essays/fixing-unix-linux-filenames.html Signed-off-by: Adam Borowski --- I really care mostly about \n but went bold and submitted a version with all ASCII control characters. If you guys disagree, please consider banning at least \n. fs/namei.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/fs/namei.c b/fs/namei.c index c75ea03ca147..8c6ed785fd49 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2817,6 +2817,18 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir) return 0; } +/* Check for banned characters in file name. Called only on creation, as + * we need to allow removal/renaming/reading of existing stuff. + */ +static int is_bad_name(const char *name) +{ + const char *c; + for (c = name; *c; c++) + if ((1 <= *c && *c <= 31) || *c == 127) + return 1; + return 0; +} + /* Check whether we can create an object with dentry child in directory * dir. * 1. We can't do it if child already exists (open has special treatment for @@ -2834,6 +2846,8 @@ static inline int may_create(struct inode *dir, struct dentry *child) return -EEXIST; if (IS_DEADDIR(dir)) return -ENOENT; + if (is_bad_name(child->d_name.name)) + return -EACCES; s_user_ns = dir->i_sb->s_user_ns; if (!kuid_has_mapping(s_user_ns, current_fsuid()) || !kgid_has_mapping(s_user_ns, current_fsgid())) @@ -2996,6 +3010,9 @@ static int may_o_create(const struct path *dir, struct dentry *dentry, umode_t m if (error) return error; + if (is_bad_name(dentry->d_name.name)) + return -EACCES; + s_user_ns = dir->dentry->d_sb->s_user_ns; if (!kuid_has_mapping(s_user_ns, current_fsuid()) || !kgid_has_mapping(s_user_ns, current_fsgid())) -- 2.14.2