From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FROM,MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H2 autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 20866 invoked from network); 1 Apr 2023 05:08:47 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 1 Apr 2023 05:08:47 -0000 Received: (qmail 24550 invoked by uid 550); 1 Apr 2023 05:08:45 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 24510 invoked from network); 1 Apr 2023 05:08:44 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680325712; x=1682917712; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=jDZV04waJz7ckc9+JvSWa26MxNWPnn8G0nh1fuQ737E=; b=jI1UTr3u2h2nVz4pXtuSjYljZoNZStPmNLMBup3LbSeP/HibO5+xW/lfqjMLHWjjKO V9Wc1Q+TFtrDXucrI6K3d+ApoW2gxRc51cLXH6lLSt2uJ1MkwqvE/FrksbtuhH8acVAv pOfLP68K97fCmIo81mVm+0fV3LwaOFXfDtF9fQfBHq14akoDXZQFozqXNr2Q62cm2rX1 Cej5D/uQzhl8K1PZMq853K9R6pBJLS7qjUgwax/QIcC0VWi1nAi3ozKDD9JOsIpOgEaM RaPujN6Mpbf66b1ThsccCqnXK2Hgw0c8DHrk6glW84ysGdSryHc8sA4LqI2fzHK2bHsv r5pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680325712; x=1682917712; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=jDZV04waJz7ckc9+JvSWa26MxNWPnn8G0nh1fuQ737E=; b=YnEGbK1WG6dFqKTUsGfWoKEyBwcYEnMq1aRSan6dx54cbZPgS4AVPE9gqc4K2CSpav 19eL1Xd91gvazyaEZbX2VEAdnp5of63JKymjNfmJEpSv0ymeyM/8nfHO/VnzqgHGCw4b vWgvpuRNXows9xBy4XydJKsLpzVt17GfJcdV8bcRLcmmTnyCOaWOyC4ahleVXCMvxcSp YUV/c9H5iYx4Y+Q51mdp9pp+BqtkYXkaHRp+BeuhwS6aezG/iOSsH1D4gY0A5na0XtEI eW/ZRRa4/WWs87PZGprgTMYnn147onT1TYUSipKXHgsk+kvvQi+qT85GgyWTG8WDAiVX p/lA== X-Gm-Message-State: AAQBX9enDOLEsmnGsyVrvj5HJEhZT2jMHHotEiyafM0PX9No79bLUxp7 ZirSkRPiKyJOov4stdUqRwYWAi+8Wv5fwQ== X-Google-Smtp-Source: AKy350YpgH/fr4fBPX4MvUXBtohSsdHQNpGSg/vFJymGrake7S+z9ImVekN+ZaWClC27x034wMAahQ== X-Received: by 2002:aa7:86d1:0:b0:62d:e32a:8b5a with SMTP id h17-20020aa786d1000000b0062de32a8b5amr5163457pfo.2.1680325712202; Fri, 31 Mar 2023 22:08:32 -0700 (PDT) From: Matthias Goergens To: musl@lists.openwall.com Cc: Matthias Goergens Date: Sat, 1 Apr 2023 13:08:23 +0800 Message-Id: <20230401050823.2341924-1-matthias.goergens@gmail.com> X-Mailer: git-send-email 2.40.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [musl] [PATCH v3] Fix UB in getmntent_r on extremely long lines 8974ef2124118e4ed8cad7ee0534b36e5c584c4e tried to fix mishandling of extremely long lines. Here's the relevant code snippet: ``` len = strlen(linebuf); if (len > INT_MAX) continue; for (i = 0; i < sizeof n / sizeof *n; i++) n[i] = len; sscanf(linebuf, " %n%*s%n %n%*s%n %n%*s%n %n%*s%n %d %d", n, n+1, n+2, n+3, n+4, n+5, n+6, n+7, &mnt->mnt_freq, &mnt->mnt_passno); } while (linebuf[n[0]] == '#' || n[1]==len); ``` Alas, that introduced undefined behaviour: if the very first line handled in the function is extremely long, `n` stays uninitialised, and thus accessing `n[0]` and `n[1]` is UB. If we handle a few sane lines before hitting a crazy long line, we don't hit C-level undefined behaviour, but the function arguably still does the wrong thing. The documentation suggests that we could return NULL on failure, but Rich Felker explained that skipping extremely long lines makes more sense here. So that's what we do. --- Note: Version 2 had a bug where it accidentally used `len > INT_MAX` instead of `len >= INT_MAX`. Please pardon the premature submission. --- src/misc/mntent.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/src/misc/mntent.c b/src/misc/mntent.c index d404fbe3..2e45c578 100644 --- a/src/misc/mntent.c +++ b/src/misc/mntent.c @@ -29,21 +29,29 @@ struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int bufle mnt->mnt_passno = 0; do { - if (use_internal) { - getline(&internal_buf, &internal_bufsize, f); - linebuf = internal_buf; - } else { - fgets(linebuf, buflen, f); - } - if (feof(f) || ferror(f)) return 0; - if (!strchr(linebuf, '\n')) { - fscanf(f, "%*[^\n]%*[\n]"); - errno = ERANGE; - return 0; - } + do { + if (use_internal) { + getline(&internal_buf, &internal_bufsize, f); + linebuf = internal_buf; + } else { + fgets(linebuf, buflen, f); + } + if (feof(f) || ferror(f)) return 0; + if (!strchr(linebuf, '\n')) { + fscanf(f, "%*[^\n]%*[\n]"); + errno = ERANGE; + return 0; + } + len = strlen(linebuf); + // In theory, with `use_internal` we could read a line longer than + // INT_MAX. But we don't want to incentivise using the legacy + // thread-unsafe API (`getmntent`). - len = strlen(linebuf); - if (len > INT_MAX) continue; + // The thread-safe API of getmntent_r only supports lengths up to + // INT_MAX, because of `int buflen` in the function signature. + + // As a compromise, we skip extremely long lines. + } while (len >= INT_MAX); for (i = 0; i < sizeof n / sizeof *n; i++) n[i] = len; sscanf(linebuf, " %n%*s%n %n%*s%n %n%*s%n %n%*s%n %d %d", n, n+1, n+2, n+3, n+4, n+5, n+6, n+7, -- 2.40.0