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 20397 invoked from network); 1 Apr 2023 05:04:34 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 1 Apr 2023 05:04:34 -0000 Received: (qmail 20137 invoked by uid 550); 1 Apr 2023 05:04:30 -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 20102 invoked from network); 1 Apr 2023 05:04:30 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680325457; x=1682917457; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=RmJF0F1CM01VkEq2OWSZvYMxxZ9Mui8sr4K4oxhzHLc=; b=h2TB7KxagTYNL9zZJqkqK+werPRpSfDawlir5+r94ZvjtK9Odumksg6RsPL4cY7aTD S5t31IG9a5S61H4oW8Y94qf+u7qmvC+MWiVpfNjeryCI9tP0MHrHQLQ0gP8BdXcuPpev 8MfUvQY8zWhweykzvRI4JAxkNp4rVQxn1QTa/1arQQwwv7Uyhkv6lXDBHqaWDv+wExry jz0C/XAb2cd4ivarJi6ZgI9pUeABTXBGOa7Rsm1WBcysku5pHmZ3nh487mukCvLYeikG 0Sr3ZxPEP94d7GPcVXzNYnmg0s5BLKuOobB2Ur9YhPeo53/2qVPHX0Q7GhtjMSSiYbC2 lh+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680325457; x=1682917457; 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=RmJF0F1CM01VkEq2OWSZvYMxxZ9Mui8sr4K4oxhzHLc=; b=BjTMuZdR/n513Xs7y6+f7JbA/LuY6dfHIUqE3LkQcqEcpJG/NZEaRQIeVPuJu6q4l5 NArLjDtm9p5bqX71IncCJL93xk5zqcxcRytMcASrP1FATo0ggKdqDOOMlTK+zbWh/0/s Nj//YPR31GNs75FCeiDz/F+e8E7o5+c/TJLMkv6YwkcG6KUm/W7Z/nILWmyvResb75Pp YJywa7WOcj+ieQXDTikKNgoe78jsh9p8BJfWSfMMXPB99+bqfpfHBvWcPFgwyfnb57jj c0GylYjATIbYquFdmYa5fs76RlBNniKA3LbURGwHiY2oriu5oRY8GVo6T+5AfJm6IhT5 WBTw== X-Gm-Message-State: AAQBX9fU8HGR7J6wC6FZVG5Hz54m7CmV+ernQsZfa3hGtstMZLY5yh22 Rt2sekOY1VqSajaZ9RKmmvDzb0KEmqVwyA== X-Google-Smtp-Source: AKy350ZI2cMy9363BM34/Yn2IPh2xhp39bZCuGDborcZ9+tqWRdcDYjh/UPEWfb0SIPjkqFXR3+68A== X-Received: by 2002:a17:902:720a:b0:1a1:b51b:4d3b with SMTP id ba10-20020a170902720a00b001a1b51b4d3bmr25603418plb.1.1680325456854; Fri, 31 Mar 2023 22:04:16 -0700 (PDT) From: Matthias Goergens To: musl@lists.openwall.com Cc: Matthias Goergens Date: Sat, 1 Apr 2023 13:04:07 +0800 Message-Id: <20230401050407.2340628-1-matthias.goergens@gmail.com> X-Mailer: git-send-email 2.40.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [musl] [PATCH v2] 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. --- 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..cdd3d0fd 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