From patchwork Thu Nov 9 13:51:18 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Christian_G=C3=B6ttsche?= X-Patchwork-Id: 13451080 X-Patchwork-Delegate: plautrba@redhat.com Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 63B5DC4332F for ; Thu, 9 Nov 2023 13:51:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232307AbjKINvc (ORCPT ); Thu, 9 Nov 2023 08:51:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43340 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231214AbjKINvb (ORCPT ); Thu, 9 Nov 2023 08:51:31 -0500 Received: from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com [IPv6:2a00:1450:4864:20::52b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A87081718 for ; Thu, 9 Nov 2023 05:51:28 -0800 (PST) Received: by mail-ed1-x52b.google.com with SMTP id 4fb4d7f45d1cf-52bd9ddb741so1408483a12.0 for ; Thu, 09 Nov 2023 05:51:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20230601; t=1699537887; x=1700142687; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:from:to:cc:subject:date:message-id:reply-to; bh=RKVMCIRV/AuXnz+tVgEzna5dSXNCWDnPusenuvAQ9s8=; b=CMIcaBS+HO0DUz3RCOeN+qr6SQqm8ZF0VkWthHE5yQbVGBBTwUXT2ab1i5Rc+h7fyn BnkTcKIsRHC1slgC6QL+dcxgc3lvZVasF3o/Io3hPfyc4rVzyj0Xvp5PmsHoFvLazRyc 1GgtNMIvhy5NxKd9321euflsaZILV6B/TUdNhDJXVomIqTgpekXiDv95iN9eLpDfS1LF filjPik2Bk7xmJVPq5K3/gC62OshS1sF/7Reb5L4HLGnnln8XEn4FxjRl9+ldoXRfMDq HdsT41QH7ms9xXW+tfelNA6fgUMWcIrVKmcdrEpA6WvlRMWp6O0H2+Ym7cA8FbZotx9T V/4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699537887; x=1700142687; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=RKVMCIRV/AuXnz+tVgEzna5dSXNCWDnPusenuvAQ9s8=; b=css+f4W23JIF1JOGwpBf9r7/ocNjmXF3z84Xc/PyjBv+Pu9h6o0VRjeRkDf8hTYioj CFeijv+xQhq0RGeQb1tRkCJSsKZIVnthqSJz/gnJGLSGDWSUM741RjH8oSF05vzHPamw Us8HSfbXtAKuzN9riTBOUX/nSWqM2yqnTPzcRHWpXHcxkBk871Gqv+0xqwBRaUtIoTA+ Z5hbod3qwqXvG/f9qOLFz1XEHpKG65N3zJryRy4Pzl19mIzc2A/xFepvxeGL0XCfiCJG L5XmB1tDjmwUN4DsiN+Y5s6ond1LPrXdxx3zM4eBYACu+TxHhMH2eCXmU31OfQ+W/PHz hlQA== X-Gm-Message-State: AOJu0YwLNTy33eslsRKLVeUFkiEdCh1PmFDTUmT9WCRlXFROs62J/cpU DT2kVKEcFBakKND17kHLg13O9EDsIxc= X-Google-Smtp-Source: AGHT+IEGmnP/GaUif7+TXNawKs/tSv7LFscTTri/T3AR24TUE228zNQz6YRHEbwcUBLnopIWGvwCpg== X-Received: by 2002:a17:907:787:b0:9bd:ff07:a586 with SMTP id xd7-20020a170907078700b009bdff07a586mr4219973ejb.68.1699537886803; Thu, 09 Nov 2023 05:51:26 -0800 (PST) Received: from debian_development.DebianHome (dynamic-077-000-043-071.77.0.pool.telefonica.de. [77.0.43.71]) by smtp.gmail.com with ESMTPSA id b4-20020a170906038400b0099bc8bd9066sm2581530eja.150.2023.11.09.05.51.26 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Nov 2023 05:51:26 -0800 (PST) From: =?utf-8?q?Christian_G=C3=B6ttsche?= To: selinux@vger.kernel.org Subject: [PATCH v2 1/4] libsepol: use str_read() where appropriate Date: Thu, 9 Nov 2023 14:51:18 +0100 Message-ID: <20231109135121.42380-1-cgzones@googlemail.com> X-Mailer: git-send-email 2.42.0 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: selinux@vger.kernel.org Use the internal helper str_read() in more places while reading strings from a binary policy. This improves readability and helps adjusting future sanity checks on inputs in fewer places. Signed-off-by: Christian Göttsche Acked-by: James Carter --- libsepol/src/policydb.c | 199 +++++++++------------------------------- 1 file changed, 41 insertions(+), 158 deletions(-) diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c index f9537caa..f608aba4 100644 --- a/libsepol/src/policydb.c +++ b/libsepol/src/policydb.c @@ -2108,7 +2108,8 @@ static int common_read(policydb_t * p, hashtab_t h, struct policy_file *fp) goto bad; len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) + rc = str_read(&key, fp, len); + if (rc < 0) goto bad; comdatum->s.value = le32_to_cpu(buf[1]); @@ -2120,14 +2121,6 @@ static int common_read(policydb_t * p, hashtab_t h, struct policy_file *fp) goto bad; nel = le32_to_cpu(buf[3]); - key = malloc(len + 1); - if (!key) - goto bad; - rc = next_entry(key, fp, len); - if (rc < 0) - goto bad; - key[len] = 0; - for (i = 0; i < nel; i++) { if (perm_read(p, comdatum->permissions.table, fp, comdatum->permissions.nprim)) goto bad; @@ -2256,11 +2249,11 @@ static int class_read(policydb_t * p, hashtab_t h, struct policy_file *fp) goto bad; len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) + rc = str_read(&key, fp, len); + if (rc < 0) goto bad; + len2 = le32_to_cpu(buf[1]); - if (is_saturated(len2)) - goto bad; cladatum->s.value = le32_to_cpu(buf[2]); if (symtab_init(&cladatum->permissions, PERM_SYMTAB_SIZE)) @@ -2272,22 +2265,10 @@ static int class_read(policydb_t * p, hashtab_t h, struct policy_file *fp) ncons = le32_to_cpu(buf[5]); - key = malloc(len + 1); - if (!key) - goto bad; - rc = next_entry(key, fp, len); - if (rc < 0) - goto bad; - key[len] = 0; - if (len2) { - cladatum->comkey = malloc(len2 + 1); - if (!cladatum->comkey) - goto bad; - rc = next_entry(cladatum->comkey, fp, len2); + rc = str_read(&cladatum->comkey, fp, len2); if (rc < 0) goto bad; - cladatum->comkey[len2] = 0; cladatum->comdatum = hashtab_search(p->p_commons.table, cladatum->comkey); @@ -2369,21 +2350,14 @@ static int role_read(policydb_t * p, hashtab_t h, struct policy_file *fp) goto bad; len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) + rc = str_read(&key, fp, len); + if (rc < 0) goto bad; role->s.value = le32_to_cpu(buf[1]); if (policydb_has_boundary_feature(p)) role->bounds = le32_to_cpu(buf[2]); - key = malloc(len + 1); - if (!key) - goto bad; - rc = next_entry(key, fp, len); - if (rc < 0) - goto bad; - key[len] = 0; - if (ebitmap_read(&role->dominates, fp)) goto bad; @@ -2460,9 +2434,6 @@ static int type_read(policydb_t * p, hashtab_t h, struct policy_file *fp) goto bad; len = le32_to_cpu(buf[pos]); - if (zero_or_saturated(len)) - goto bad; - typdatum->s.value = le32_to_cpu(buf[++pos]); if (policydb_has_boundary_feature(p)) { uint32_t properties; @@ -2503,13 +2474,9 @@ static int type_read(policydb_t * p, hashtab_t h, struct policy_file *fp) goto bad; } - key = malloc(len + 1); - if (!key) - goto bad; - rc = next_entry(key, fp, len); + rc = str_read(&key, fp, len); if (rc < 0) goto bad; - key[len] = 0; if (hashtab_insert(h, key, typdatum)) goto bad; @@ -2681,14 +2648,8 @@ static int filename_trans_read_one_compat(policydb_t *p, struct policy_file *fp) if (rc < 0) return -1; len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) - return -1; - - name = calloc(len + 1, sizeof(*name)); - if (!name) - return -1; - rc = next_entry(name, fp, len); + rc = str_read(&name, fp, len); if (rc < 0) goto err; @@ -2766,14 +2727,8 @@ static int filename_trans_read_one(policydb_t *p, struct policy_file *fp) if (rc < 0) return -1; len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) - return -1; - - name = calloc(len + 1, sizeof(*name)); - if (!name) - return -1; - rc = next_entry(name, fp, len); + rc = str_read(&name, fp, len); if (rc < 0) goto err; @@ -2957,16 +2912,9 @@ static int ocontext_read_xen(const struct policydb_compat_info *info, if (rc < 0) return -1; len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) - return -1; - - c->u.name = malloc(len + 1); - if (!c->u.name) - return -1; - rc = next_entry(c->u.name, fp, len); + rc = str_read(&c->u.name, fp, len); if (rc < 0) return -1; - c->u.name[len] = 0; if (context_read_and_validate (&c->context[0], p, fp)) return -1; @@ -3024,15 +2972,13 @@ static int ocontext_read_selinux(const struct policydb_compat_info *info, if (rc < 0) return -1; len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len) || len > 63) + if (len > 63) return -1; - c->u.name = malloc(len + 1); - if (!c->u.name) - return -1; - rc = next_entry(c->u.name, fp, len); + + rc = str_read(&c->u.name, fp, len); if (rc < 0) return -1; - c->u.name[len] = 0; + if (context_read_and_validate (&c->context[0], p, fp)) return -1; @@ -3080,13 +3026,10 @@ static int ocontext_read_selinux(const struct policydb_compat_info *info, if (port > UINT8_MAX || port == 0) return -1; - c->u.ibendport.dev_name = malloc(len + 1); - if (!c->u.ibendport.dev_name) - return -1; - rc = next_entry(c->u.ibendport.dev_name, fp, len); + rc = str_read(&c->u.ibendport.dev_name, fp, len); if (rc < 0) return -1; - c->u.ibendport.dev_name[len] = 0; + c->u.ibendport.port = port; if (context_read_and_validate (&c->context[0], p, fp)) @@ -3120,15 +3063,11 @@ static int ocontext_read_selinux(const struct policydb_compat_info *info, return -1; c->v.behavior = le32_to_cpu(buf[0]); len = le32_to_cpu(buf[1]); - if (zero_or_saturated(len)) - return -1; - c->u.name = malloc(len + 1); - if (!c->u.name) - return -1; - rc = next_entry(c->u.name, fp, len); + + rc = str_read(&c->u.name, fp, len); if (rc < 0) return -1; - c->u.name[len] = 0; + if (context_read_and_validate (&c->context[0], p, fp)) return -1; @@ -3196,23 +3135,17 @@ static int genfs_read(policydb_t * p, struct policy_file *fp) if (rc < 0) goto bad; len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) - goto bad; newgenfs = calloc(1, sizeof(genfs_t)); if (!newgenfs) goto bad; - newgenfs->fstype = malloc(len + 1); - if (!newgenfs->fstype) { - free(newgenfs); - goto bad; - } - rc = next_entry(newgenfs->fstype, fp, len); + + rc = str_read(&newgenfs->fstype, fp, len); if (rc < 0) { free(newgenfs->fstype); free(newgenfs); goto bad; } - newgenfs->fstype[len] = 0; + for (genfs_p = NULL, genfs = p->genfs; genfs; genfs_p = genfs, genfs = genfs->next) { if (strcmp(newgenfs->fstype, genfs->fstype) == 0) { @@ -3243,16 +3176,10 @@ static int genfs_read(policydb_t * p, struct policy_file *fp) if (rc < 0) goto bad; len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) - goto bad; - newc->u.name = malloc(len + 1); - if (!newc->u.name) { - goto bad; - } - rc = next_entry(newc->u.name, fp, len); + rc = str_read(&newc->u.name, fp, len); if (rc < 0) goto bad; - newc->u.name[len] = 0; + rc = next_entry(buf, fp, sizeof(uint32_t)); if (rc < 0) goto bad; @@ -3344,21 +3271,14 @@ static int user_read(policydb_t * p, hashtab_t h, struct policy_file *fp) goto bad; len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) + rc = str_read(&key, fp, len); + if (rc < 0) goto bad; usrdatum->s.value = le32_to_cpu(buf[1]); if (policydb_has_boundary_feature(p)) usrdatum->bounds = le32_to_cpu(buf[2]); - key = malloc(len + 1); - if (!key) - goto bad; - rc = next_entry(key, fp, len); - if (rc < 0) - goto bad; - key[len] = 0; - if (p->policy_type == POLICY_KERN) { if (ebitmap_read(&usrdatum->roles.roles, fp)) goto bad; @@ -3430,19 +3350,12 @@ static int sens_read(policydb_t * p goto bad; len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) + rc = str_read(&key, fp, len); + if (rc < 0) goto bad; levdatum->isalias = le32_to_cpu(buf[1]); - key = malloc(len + 1); - if (!key) - goto bad; - rc = next_entry(key, fp, len); - if (rc < 0) - goto bad; - key[len] = 0; - levdatum->level = malloc(sizeof(mls_level_t)); if (!levdatum->level || mls_read_level(levdatum->level, fp)) goto bad; @@ -3476,20 +3389,13 @@ static int cat_read(policydb_t * p goto bad; len = le32_to_cpu(buf[0]); - if(zero_or_saturated(len)) + rc = str_read(&key, fp, len); + if (rc < 0) goto bad; catdatum->s.value = le32_to_cpu(buf[1]); catdatum->isalias = le32_to_cpu(buf[2]); - key = malloc(len + 1); - if (!key) - goto bad; - rc = next_entry(key, fp, len); - if (rc < 0) - goto bad; - key[len] = 0; - if (hashtab_insert(h, key, catdatum)) goto bad; @@ -3865,17 +3771,10 @@ static int filename_trans_rule_read(policydb_t *p, filename_trans_rule_t **r, return -1; len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) - return -1; - ftr->name = malloc(len + 1); - if (!ftr->name) - return -1; - - rc = next_entry(ftr->name, fp, len); - if (rc) + rc = str_read(&ftr->name, fp, len); + if (rc < 0) return -1; - ftr->name[len] = 0; if (type_set_read(&ftr->stypes, fp)) return -1; @@ -4119,15 +4018,10 @@ static int scope_read(policydb_t * p, int symnum, struct policy_file *fp) if (rc < 0) goto cleanup; key_len = le32_to_cpu(buf[0]); - if (zero_or_saturated(key_len)) - goto cleanup; - key = malloc(key_len + 1); - if (!key) - goto cleanup; - rc = next_entry(key, fp, key_len); + + rc = str_read(&key, fp, key_len); if (rc < 0) goto cleanup; - key[key_len] = '\0'; /* ensure that there already exists a symbol with this key */ if (hashtab_search(p->symtab[symnum].table, key) == NULL) { @@ -4387,28 +4281,17 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose) goto bad; } len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) - goto bad; - if ((p->name = malloc(len + 1)) == NULL) { - goto bad; - } - if ((rc = next_entry(p->name, fp, len)) < 0) { + rc = str_read(&p->name, fp, len); + if (rc < 0) goto bad; - } - p->name[len] = '\0'; + if ((rc = next_entry(buf, fp, sizeof(uint32_t))) < 0) { goto bad; } len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) - goto bad; - if ((p->version = malloc(len + 1)) == NULL) { - goto bad; - } - if ((rc = next_entry(p->version, fp, len)) < 0) { + rc = str_read(&p->version, fp, len); + if (rc < 0) goto bad; - } - p->version[len] = '\0'; } if ((p->policyvers >= POLICYDB_VERSION_POLCAP && From patchwork Thu Nov 9 13:51:19 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Christian_G=C3=B6ttsche?= X-Patchwork-Id: 13451081 X-Patchwork-Delegate: plautrba@redhat.com Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 19E15C4167B for ; Thu, 9 Nov 2023 13:51:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234085AbjKINvc (ORCPT ); Thu, 9 Nov 2023 08:51:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43348 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232167AbjKINvc (ORCPT ); Thu, 9 Nov 2023 08:51:32 -0500 Received: from mail-ej1-x630.google.com (mail-ej1-x630.google.com [IPv6:2a00:1450:4864:20::630]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 639D72702 for ; Thu, 9 Nov 2023 05:51:29 -0800 (PST) Received: by mail-ej1-x630.google.com with SMTP id a640c23a62f3a-9dd6dc9c00cso148102966b.3 for ; Thu, 09 Nov 2023 05:51:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20230601; t=1699537887; x=1700142687; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:from:to:cc:subject:date:message-id :reply-to; bh=7bm45oeM6k4PguQzjWqSeyghD0z8zmKPrFsjAQdnirU=; b=Gnlx6oUHhRJLBOjC+ccwqdYnA2oyqZUz/3xxh1rb9S6U/jWFmYzEYGRAXpy3m8B/CW WdAoszwtWUrCov2CLgGpibF2AkAPO2woi6DShwIqxBdh1orIBpL543UfN0tLWBu2OJWz F9aIMeS543H7WgexGr+5Tp/q6mY7E736p7h+/oWWAJaicyx+uH5nqTuLo3E/OPQY5SIh 3oA55oL6m1PEVMWNM7cO0b13AEWh8uxv+LvAeYD/V537J8+O6bSnqOPxUVd8BsHkCl1Q yfcbSoFuOeTRvknlbOmUddIab+TGmkavBjHtLUPoSUXUpVN7iFeFOqR2ujpCRdWOJikK KWGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699537887; x=1700142687; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=7bm45oeM6k4PguQzjWqSeyghD0z8zmKPrFsjAQdnirU=; b=MtR/cZ6i8VgyzXJcTKm9fRJMjrlUgBcnhL7cL9rLLKRcjD8CY2XfbvP+GPy8rnEa0z 9bfpgMZJRWCG37R0fD+Wftpu+6zDv8Xv9KGn+HUkonuipNQpkZYOZIvW1Kr+oI/iazq6 P2WHOgATvRjVPk06Ft8ivLXhJS/J1JdB1ZPHZk+ERyZ339KX55VhvVtBLumR1VGCkUZd IMuNEiiSYdBkIMt32/+UTFlPoAYHyj2gsBO6RWNhLHlI3d2nDGQV2mGRzAZxpSdJSg98 8XoDJppE3840Vvk6tprQCr6TvNBzmybRYa45K5miVnZ9eCw9WGJeE1XqZsyBo6Pojeiv gfsw== X-Gm-Message-State: AOJu0YyuqKIUOFHJAFPTIeBTNq+/z+mN2rG5Fuxek7ksuNTMNQ7FBm5S 4+YY0lSCFC0L/qawIeHN/SYhsDWHjc8= X-Google-Smtp-Source: AGHT+IFLPk/kxNFhoDSeBQ3Au6TsQo16VPLzYgHR82kRUwbAIbaDjDgPdMX7Xtw7ZDui2qWtjXBuCg== X-Received: by 2002:a17:907:8d87:b0:9e3:f99a:3061 with SMTP id tf7-20020a1709078d8700b009e3f99a3061mr3475489ejc.44.1699537887336; Thu, 09 Nov 2023 05:51:27 -0800 (PST) Received: from debian_development.DebianHome (dynamic-077-000-043-071.77.0.pool.telefonica.de. [77.0.43.71]) by smtp.gmail.com with ESMTPSA id b4-20020a170906038400b0099bc8bd9066sm2581530eja.150.2023.11.09.05.51.26 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Nov 2023 05:51:27 -0800 (PST) From: =?utf-8?q?Christian_G=C3=B6ttsche?= To: selinux@vger.kernel.org Subject: [PATCH v2 2/4] libsepol: adjust type for saturation check Date: Thu, 9 Nov 2023 14:51:19 +0100 Message-ID: <20231109135121.42380-2-cgzones@googlemail.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231109135121.42380-1-cgzones@googlemail.com> References: <20231109135121.42380-1-cgzones@googlemail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: selinux@vger.kernel.org Change the type for the number of primary names in a symtab to uint32_t, which conforms to the bytes read and the type used in the symtab. The type is important for the saturation check via is_saturated(), since it checks against -1 casted to the specific type. Signed-off-by: Christian Göttsche --- v2: Split from subsequent patch --- libsepol/src/policydb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c index f608aba4..bc7bc9dc 100644 --- a/libsepol/src/policydb.c +++ b/libsepol/src/policydb.c @@ -4120,8 +4120,8 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose) { unsigned int i, j, r_policyvers; - uint32_t buf[5]; - size_t len, nprim, nel; + uint32_t buf[5], nprim; + size_t len, nel; char *policydb_str; const struct policydb_compat_info *info; unsigned int policy_type, bufindex; From patchwork Thu Nov 9 13:51:20 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Christian_G=C3=B6ttsche?= X-Patchwork-Id: 13451083 X-Patchwork-Delegate: plautrba@redhat.com Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A0745C4167D for ; Thu, 9 Nov 2023 13:51:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232167AbjKINve (ORCPT ); Thu, 9 Nov 2023 08:51:34 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43354 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231214AbjKINvc (ORCPT ); Thu, 9 Nov 2023 08:51:32 -0500 Received: from mail-ej1-x632.google.com (mail-ej1-x632.google.com [IPv6:2a00:1450:4864:20::632]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D4BAC2D5B for ; Thu, 9 Nov 2023 05:51:29 -0800 (PST) Received: by mail-ej1-x632.google.com with SMTP id a640c23a62f3a-9e2838bcb5eso149350566b.0 for ; Thu, 09 Nov 2023 05:51:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20230601; t=1699537888; x=1700142688; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:from:to:cc:subject:date:message-id :reply-to; bh=wKRhSWjN+2wNUbQcTfizTF9WbNhyKNFAuMWdEbj3f84=; b=TdRl51R2xybSzkRGtba3BUYYtjM5xP6UaaYNb3Kg80RPVzMpysxDp/Tza2QWW21mr8 +w0SkisOpOGBtDyiq+S8U8o/QgaomDbitm2i4mQr6bRM1W8JCEfWX7GNcp8iJSVf5BxB TmBmUQaXjcsS1aK2MFZdHyiwGjTGxo5zaQWiuDCvnpGxwI9NGaN2XfhgSlDNV5Ad0Mft AAtRmcASMsVmR4K3t3kM1IIcBJ/qPvKRwFknZuNGLJOdK9YkkwMK1kByAfZNjcVT2LmX rrZ5q4/BPOaFWJvcF5Xk5IJ93X8in1gSRyGotbs47O9aFz54j9DymBm0Sd8lSVvxBqPt rIyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699537888; x=1700142688; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=wKRhSWjN+2wNUbQcTfizTF9WbNhyKNFAuMWdEbj3f84=; b=M0SCXGbUl+M8Qgeek7QVsgWTbvlUtsc26KK8IfHtXBc9whARPKLkLPb6I5w+yqvmYQ 6NOLTD1Krf9I3RHn/up/QFkIlifJNnhtdSiZvcv6/QJWfODevKK8g4+XZBgSsnKvUFZh tm0bx7wOJeTVsBHremuUGzD2vR8Gq/kZ76NUk9YTkkfaxm9/YU5y42JeRd1BUwY3U9y/ fWfG4J/2t+7IckEaSH9GeYJJQteLbmadpO1rW6gpxEk1VOX7OWlTmu1H3+9+aKtw34Nz Qi+T69NWf2yOa3LbA2WBnrEC3zHn05SYntIsbEoxcGT37kbiuK179lH2eTh0wdKqzvOR 70BQ== X-Gm-Message-State: AOJu0YyuZqpkg7+WEHvmdZoUcPcixmI9xWrsPCkU26bAJQxuEjo765f1 /9r7PQJqIERNEgRKqUeeREGfy9b2Ql8= X-Google-Smtp-Source: AGHT+IE9bCluVNmroDGnFW3s6mmd8hfdoudvRYQLOv7Qlg6EPZyNm2KxIp7/5E4D4VcQtF43wnqNEA== X-Received: by 2002:a17:907:9304:b0:9d3:85b9:afdf with SMTP id bu4-20020a170907930400b009d385b9afdfmr4330176ejc.3.1699537888082; Thu, 09 Nov 2023 05:51:28 -0800 (PST) Received: from debian_development.DebianHome (dynamic-077-000-043-071.77.0.pool.telefonica.de. [77.0.43.71]) by smtp.gmail.com with ESMTPSA id b4-20020a170906038400b0099bc8bd9066sm2581530eja.150.2023.11.09.05.51.27 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Nov 2023 05:51:27 -0800 (PST) From: =?utf-8?q?Christian_G=C3=B6ttsche?= To: selinux@vger.kernel.org Subject: [PATCH v2 3/4] libsepol: enhance saturation check Date: Thu, 9 Nov 2023 14:51:20 +0100 Message-ID: <20231109135121.42380-3-cgzones@googlemail.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231109135121.42380-1-cgzones@googlemail.com> References: <20231109135121.42380-1-cgzones@googlemail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: selinux@vger.kernel.org Several values while parsing kernel policies, like symtab sizes or string lengths, are checked for saturation. They may not be set to the maximum value, to avoid overflows or occupying a reserved value, and many of those sizes must not be 0. This is currently handled via the two macros is_saturated() and zero_or_saturated(). Both macros are tweaked for the fuzzer, because the fuzzer can create input with huge sizes. While there is no subsequent data to provide the announced sizes, which will be caught later, memory of the requested size is allocated, which would lead to OOM reports. Thus the sizes for the fuzzer are limited to 2^16. This has the drawback of the fuzzer not checking the complete input space. Check the sizes in question for actual enough bytes available in the input. This is (only) possible for mapped memory, which the fuzzer uses. Application like setools do currently not benefit from this change, since they load the policy via a stream. There are currently multiple interfaces to load a policy, so reworking them to use mapped memory by default might be subject for future work. Signed-off-by: Christian Göttsche --- v2: keep is_saturated() and zero_or_saturated() unchanged --- libsepol/src/avtab.c | 2 +- libsepol/src/policydb.c | 9 ++++++--- libsepol/src/private.h | 22 ++++++++++++++++------ libsepol/src/services.c | 2 +- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/libsepol/src/avtab.c b/libsepol/src/avtab.c index 7c2328b7..b2fa8d85 100644 --- a/libsepol/src/avtab.c +++ b/libsepol/src/avtab.c @@ -600,7 +600,7 @@ int avtab_read(avtab_t * a, struct policy_file *fp, uint32_t vers) goto bad; } nel = le32_to_cpu(buf[0]); - if (zero_or_saturated(nel)) { + if (zero_or_saturated(nel) || exceeds_available_bytes(fp, nel, sizeof(uint32_t) * 3)) { ERR(fp->handle, "table is empty"); goto bad; } diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c index bc7bc9dc..6ba4f916 100644 --- a/libsepol/src/policydb.c +++ b/libsepol/src/policydb.c @@ -3857,7 +3857,8 @@ static int scope_index_read(scope_index_t * scope_index, if (rc < 0) return -1; scope_index->class_perms_len = le32_to_cpu(buf[0]); - if (is_saturated(scope_index->class_perms_len)) + if (is_saturated(scope_index->class_perms_len) || + exceeds_available_bytes(fp, scope_index->class_perms_len, sizeof(uint32_t) * 3)) return -1; if (scope_index->class_perms_len == 0) { scope_index->class_perms_map = NULL; @@ -4036,7 +4037,8 @@ static int scope_read(policydb_t * p, int symnum, struct policy_file *fp) goto cleanup; scope->scope = le32_to_cpu(buf[0]); scope->decl_ids_len = le32_to_cpu(buf[1]); - if (zero_or_saturated(scope->decl_ids_len)) { + if (zero_or_saturated(scope->decl_ids_len) || + exceeds_available_bytes(fp, scope->decl_ids_len, sizeof(uint32_t))) { ERR(fp->handle, "invalid scope with no declaration"); goto cleanup; } @@ -4315,7 +4317,8 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose) if (rc < 0) goto bad; nprim = le32_to_cpu(buf[0]); - if (is_saturated(nprim)) + if (is_saturated(nprim) || + exceeds_available_bytes(fp, nprim, sizeof(uint32_t) * 3)) goto bad; nel = le32_to_cpu(buf[1]); if (nel && !nprim) { diff --git a/libsepol/src/private.h b/libsepol/src/private.h index 1833b497..1500bbc2 100644 --- a/libsepol/src/private.h +++ b/libsepol/src/private.h @@ -44,13 +44,23 @@ #define ARRAY_SIZE(x) (sizeof(x)/sizeof((x)[0])) -#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -# define is_saturated(x) (x == (typeof(x))-1 || (x) > (1U << 16)) -#else -# define is_saturated(x) (x == (typeof(x))-1) -#endif +static inline int exceeds_available_bytes(const struct policy_file *fp, size_t x, size_t req_elem_size) +{ + size_t req_size; + + /* Remaining input size is only available for mmap'ed memory */ + if (fp->type != PF_USE_MEMORY) + return 0; + + if (__builtin_mul_overflow(x, req_elem_size, &req_size)) + return 1; + + return req_size > fp->len; +} + +#define is_saturated(x) ((x) == (typeof(x))-1) -#define zero_or_saturated(x) ((x == 0) || is_saturated(x)) +#define zero_or_saturated(x) (((x) == 0) || is_saturated(x)) #define spaceship_cmp(a, b) (((a) > (b)) - ((a) < (b))) diff --git a/libsepol/src/services.c b/libsepol/src/services.c index 51bd56a0..aa1ad52c 100644 --- a/libsepol/src/services.c +++ b/libsepol/src/services.c @@ -1748,7 +1748,7 @@ int str_read(char **strp, struct policy_file *fp, size_t len) int rc; char *str; - if (zero_or_saturated(len)) { + if (zero_or_saturated(len) || exceeds_available_bytes(fp, len, sizeof(char))) { errno = EINVAL; return -1; } From patchwork Thu Nov 9 13:51:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Christian_G=C3=B6ttsche?= X-Patchwork-Id: 13451082 X-Patchwork-Delegate: plautrba@redhat.com Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 37D65C41535 for ; Thu, 9 Nov 2023 13:51:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231214AbjKINve (ORCPT ); Thu, 9 Nov 2023 08:51:34 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43364 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234055AbjKINvc (ORCPT ); Thu, 9 Nov 2023 08:51:32 -0500 Received: from mail-ej1-x62d.google.com (mail-ej1-x62d.google.com [IPv6:2a00:1450:4864:20::62d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 472AE2D65 for ; Thu, 9 Nov 2023 05:51:30 -0800 (PST) Received: by mail-ej1-x62d.google.com with SMTP id a640c23a62f3a-9e4675c7a5fso145276466b.0 for ; Thu, 09 Nov 2023 05:51:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20230601; t=1699537889; x=1700142689; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:from:to:cc:subject:date:message-id :reply-to; bh=rXM1yzHhZfeCdqppdDUd/j5PIuOJdhbrlyNG9DBRlX4=; b=DuqYcwiPRNfJvYEfjCHt/uYjb1kZIlxZY63I+nCW6Sf/+QGmHaKpE+LWUMSzJuN63d Ls8NJ3KjERpxZEHjDDYvr/kLjZXDvLScxzpzrczvsnn9MWhmfgIrQXwsxobe7a2vmVcs Mkne8iQklAuAh/hXN42fO+jY90658hqshgWETE0vzgEfoA8IuJxlOUR4uuQ44p6/GbpV TnXC9vVMMrzhVToCP/Cw6OHXQN2vbDe5ThzlJPH5BJhFI4yuBg/aGAxMfHtW2/Vr9vn+ 92qGH83lNtKPD7LqIxoMU6wuMwcd5WErs88O4vzxL00dQpNY7l1YpnAAk8B5FH2q63Xc 2G5A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699537889; x=1700142689; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=rXM1yzHhZfeCdqppdDUd/j5PIuOJdhbrlyNG9DBRlX4=; b=aKAegxrPRiM4KDpzPu5smba9h5+0x4f34YbPY8qFJzA8i/qMRj/5mYGfPea1PATFhh abvwA7PZAbxia/9adpgeWNOgKLgl4DUeJWixalI5Jmt6shWf1bpTzQ49e4S8Z3DYKttG m7ZTcSfh1W5M96ee/431Cgj/I2tdDW25b3tZjqCJpUgzQbzpnNgp03KHQIo6AFAPgwti ADCnusgs9WvEZSkVdgelOmTtatVycu2KlTVKm0hdH05th9NZ8EivvClfKDSucHKBxGcR JwbS3+HYaDjnyCphIoaWqLBFm/ODnvMoH9i7PDym0Z1wy/mdZz1qaZqg4qN4AF59qMy7 5lSA== X-Gm-Message-State: AOJu0YxbzF9ykH1/J/l/FandK8JjdPhsUm6g68JD7tdwS52EFiQzoPby tHpzSFslNmQQNkKs5sIsMKIMyVxDkXw= X-Google-Smtp-Source: AGHT+IGvXugR9bf2utR7Zm+APQOtDKnP3AzIGTEEAlZN2TRiCriuWlBFnHEUGn+PnvJo7gO/CCItBw== X-Received: by 2002:a17:907:744:b0:9de:32bb:faab with SMTP id xc4-20020a170907074400b009de32bbfaabmr4149154ejb.32.1699537888576; Thu, 09 Nov 2023 05:51:28 -0800 (PST) Received: from debian_development.DebianHome (dynamic-077-000-043-071.77.0.pool.telefonica.de. [77.0.43.71]) by smtp.gmail.com with ESMTPSA id b4-20020a170906038400b0099bc8bd9066sm2581530eja.150.2023.11.09.05.51.28 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Nov 2023 05:51:28 -0800 (PST) From: =?utf-8?q?Christian_G=C3=B6ttsche?= To: selinux@vger.kernel.org Subject: [PATCH v2 4/4] libsepol: validate the identifier for initials SID is valid Date: Thu, 9 Nov 2023 14:51:21 +0100 Message-ID: <20231109135121.42380-4-cgzones@googlemail.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231109135121.42380-1-cgzones@googlemail.com> References: <20231109135121.42380-1-cgzones@googlemail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: selinux@vger.kernel.org Check the identifier for initial SIDs is less than the maximum known ID. The kernel will ignore all unknown IDs, see security/selinux/ss/policydb.c:policydb_load_isids(). Without checking huge IDs result in OOM events, while writing policies, e.g. in write_sids_to_conf() or write_sids_to_cil(), due to allocation of large (continuous) string lists. Signed-off-by: Christian Göttsche --- v2: check while validation against actual number of ISIDs --- libsepol/src/policydb_validate.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c index 016ab655..32ad5a18 100644 --- a/libsepol/src/policydb_validate.c +++ b/libsepol/src/policydb_validate.c @@ -6,6 +6,7 @@ #include #include "debug.h" +#include "kernel_to_common.h" #include "policydb_validate.h" #define bool_xor(a, b) (!(a) != !(b)) @@ -1180,6 +1181,10 @@ static int validate_ocontexts(sepol_handle_t *handle, const policydb_t *p, valid if (p->target_platform == SEPOL_TARGET_SELINUX) { switch (i) { + case OCON_ISID: + if (octx->sid[0] == SEPOL_SECSID_NULL || octx->sid[0] >= SELINUX_SID_SZ) + goto bad; + break; case OCON_FS: case OCON_NETIF: if (validate_context(&octx->context[1], flavors, p->mls)) @@ -1216,6 +1221,10 @@ static int validate_ocontexts(sepol_handle_t *handle, const policydb_t *p, valid } } else if (p->target_platform == SEPOL_TARGET_XEN) { switch(i) { + case OCON_XEN_ISID: + if (octx->sid[0] == SEPOL_SECSID_NULL || octx->sid[0] >= XEN_SID_SZ) + goto bad; + break; case OCON_XEN_IOPORT: if (octx->u.ioport.low_ioport > octx->u.ioport.high_ioport) goto bad;