From patchwork Tue Oct 3 00:50:42 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adam Borowski X-Patchwork-Id: 9981375 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A8CCC60384 for ; Tue, 3 Oct 2017 00:51:10 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9CA35286AC for ; Tue, 3 Oct 2017 00:51:10 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 91895288A6; Tue, 3 Oct 2017 00:51:10 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 41CF4286AC for ; Tue, 3 Oct 2017 00:51:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750984AbdJCAuz (ORCPT ); Mon, 2 Oct 2017 20:50:55 -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 Received: from 89-71-161-30.dynamic.chello.pl ([89.71.161.30] helo=umbar.angband.pl) by tartarus.angband.pl with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1dzBQ8-0003zh-CW; Tue, 03 Oct 2017 02:50:51 +0200 Received: from kilobyte by umbar.angband.pl with local (Exim 4.89) (envelope-from ) id 1dzBQ6-0004IX-Io; Tue, 03 Oct 2017 02:50:46 +0200 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-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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()))