From patchwork Fri Jul 6 05:25:00 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 10510597 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 A8D3460532 for ; Fri, 6 Jul 2018 05:26:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 982BB2837E for ; Fri, 6 Jul 2018 05:26:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8BE32283C8; Fri, 6 Jul 2018 05:26:18 +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 341622837E for ; Fri, 6 Jul 2018 05:26:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932205AbeGFFZy (ORCPT ); Fri, 6 Jul 2018 01:25:54 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:45689 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932197AbeGFFZv (ORCPT ); Fri, 6 Jul 2018 01:25:51 -0400 Received: from 162-237-133-238.lightspeed.rcsntx.sbcglobal.net ([162.237.133.238] helo=sec.l.tihix.com) by youngberry.canonical.com with esmtpsa (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1fbJFd-0001Jc-Vu; Fri, 06 Jul 2018 05:25:50 +0000 From: Tyler Hicks To: John Johansen Cc: James Morris , Serge Hallyn , Seth Arnold , linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/2] apparmor: Check buffer bounds when mapping permissions mask Date: Fri, 6 Jul 2018 05:25:00 +0000 Message-Id: <1530854701-7348-2-git-send-email-tyhicks@canonical.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1530854701-7348-1-git-send-email-tyhicks@canonical.com> References: <1530854701-7348-1-git-send-email-tyhicks@canonical.com> Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP Don't read past the end of the buffer containing permissions characters or write past the end of the destination string. Detected by CoverityScan CID#1415361, 1415376 ("Out-of-bounds access") Fixes: e53cfe6c7caa ("apparmor: rework perm mapping to a slightly broader set") Signed-off-by: Tyler Hicks Acked-by: Serge Hallyn --- security/apparmor/file.c | 3 ++- security/apparmor/include/perms.h | 3 ++- security/apparmor/lib.c | 17 +++++++++++++---- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/security/apparmor/file.c b/security/apparmor/file.c index 224b2fef93ca..4285943f7260 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -47,7 +47,8 @@ static void audit_file_mask(struct audit_buffer *ab, u32 mask) { char str[10]; - aa_perm_mask_to_str(str, aa_file_perm_chrs, map_mask_to_chr_mask(mask)); + aa_perm_mask_to_str(str, sizeof(str), aa_file_perm_chrs, + map_mask_to_chr_mask(mask)); audit_log_string(ab, str); } diff --git a/security/apparmor/include/perms.h b/security/apparmor/include/perms.h index 38aa6247d00f..b94ec114d1a4 100644 --- a/security/apparmor/include/perms.h +++ b/security/apparmor/include/perms.h @@ -137,7 +137,8 @@ extern struct aa_perms allperms; xcheck(fn_for_each((L1), (P), (FN1)), fn_for_each((L2), (P), (FN2))) -void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask); +void aa_perm_mask_to_str(char *str, size_t str_size, const char *chrs, + u32 mask); void aa_audit_perm_names(struct audit_buffer *ab, const char * const *names, u32 mask); void aa_audit_perm_mask(struct audit_buffer *ab, u32 mask, const char *chrs, diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index a7b3f681b80e..7ab368c3789b 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -198,15 +198,24 @@ const char *aa_file_perm_names[] = { /** * aa_perm_mask_to_str - convert a perm mask to its short string * @str: character buffer to store string in (at least 10 characters) + * @str_size: size of the @str buffer + * @chrs: NUL-terminated character buffer of permission characters * @mask: permission mask to convert */ -void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask) +void aa_perm_mask_to_str(char *str, size_t str_size, const char *chrs, u32 mask) { unsigned int i, perm = 1; + size_t num_chrs = strlen(chrs); + + for (i = 0; i < num_chrs; perm <<= 1, i++) { + if (mask & perm) { + /* Ensure that one byte is left for NUL-termination */ + if (WARN_ON_ONCE(str_size <= 1)) + continue; - for (i = 0; i < 32; perm <<= 1, i++) { - if (mask & perm) *str++ = chrs[i]; + str_size--; + } } *str = '\0'; } @@ -236,7 +245,7 @@ void aa_audit_perm_mask(struct audit_buffer *ab, u32 mask, const char *chrs, audit_log_format(ab, "\""); if ((mask & chrsmask) && chrs) { - aa_perm_mask_to_str(str, chrs, mask & chrsmask); + aa_perm_mask_to_str(str, sizeof(str), chrs, mask & chrsmask); mask &= ~chrsmask; audit_log_format(ab, "%s", str); if (mask & namesmask)