From nobody Fri May 10 21:17:01 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4VbhXZ20Xbz5KRQC; Fri, 10 May 2024 21:17:02 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4VbhXY6P1Cz4c8C; Fri, 10 May 2024 21:17:01 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1715375821; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=RaIL5wjm0PUX9m8cZ/4cAkWOWDHwBA0j57zaeOOdVSc=; b=bUSyyVoFxuf9jr5MMXvqiFHHqhvphMZAxYiUBdjSLpvdRKMaZ2KubuNtJ2a2YlXXzohB8/ qpJsUtZF1LtS5KJ+m0tOOcyAv2xee7Nv1oPAtKhuzSnc67jmaLZ9go4Qri6KY6DUR/y0BP pvUSW+6RkdFzK0jkXhajzrJYwm4wucKmPj6x52OprbfYgClSIxUsXyx8jIv5KPNCG9sI+1 2KfN00nEweJjlm2EMNNJQmQOCCqIxQUJfHSfCaIMJeQcIPJdz+SMNKPTrJsvVJTlV2u+z6 MFtnTff4/2G4wZfRXlHRTqiKoIw6SWKkDHSllSpOmAHpZBXrJdNDZsAObZvB3w== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1715375821; a=rsa-sha256; cv=none; b=bew8C43uPd6VTz/91s7sddPTOx0jMdQaTP83F3IOCu6rdVgqfYPHIF7eBMzsN1B8p/GT+Q k6I9oaMjJNHpVrGBzNPeKKjeVOd4rOi+VCEilGYRZ635HmvVeOnYJiUScw6/MLpFu5z5Cp nPUf6DeMhbwnw0kogQYNslLkKPg58J5fDVh/TVLtwaKrmjudO2N58tQqO6dUqUzi1bqgH9 am9oDXKS+JkPL/au7spbgH2XkeZmBI8l+APpExcOoKEnv9VAT3TuAy+JesaQ5qusgH//+C l1b3QJSAohhytK1ymi/wUPGSSTldh5GEQ8QqUWAgEqPNBMPmYvCMIdLfMZw3tA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1715375821; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=RaIL5wjm0PUX9m8cZ/4cAkWOWDHwBA0j57zaeOOdVSc=; b=HzBMR6cyjQVWT22wtEGpyC+UGYM+2VPn+Sw0S09altZCfPC+a5uC5gNnTsr84V14uqmka+ yTRVOKrhBZlfsh3tm9Wn9OE25rbF5BdjRDS4ErMn7hajPsLgb/mmeHv1USZtBxBkPBGFVT 3k2uwW/Rjzm4N6q1Usfsp5OrfE9157x0uexmc96N6mPOEJfyrrRnhYNUJ8QXZmXCPjnlY7 owSCooJ/PNMm5eKVehPXEWSrHmI8R2u0Sx8VAwYWCwBn9+TCw3CbywFA8j5ZWMZFV6jvxm z2kcdhfqn1XP7RAZcHpU9V5bEC+1qAgIFoymaECKow1Do+g/YmeEA/DHw8gdrQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4VbhXY61PyztXV; Fri, 10 May 2024 21:17:01 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 44ALH1LZ082947; Fri, 10 May 2024 21:17:01 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 44ALH1ub082944; Fri, 10 May 2024 21:17:01 GMT (envelope-from git) Date: Fri, 10 May 2024 21:17:01 GMT Message-Id: <202405102117.44ALH1ub082944@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Dag-Erling =?utf-8?Q?Sm=C3=B8rgrav?= Subject: git: 25945af47e7a - main - tftpd: silence gcc overflow warnings List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: des X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 25945af47e7a1d06c44c8c160045a866e90569ab Auto-Submitted: auto-generated The branch main has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=25945af47e7a1d06c44c8c160045a866e90569ab commit 25945af47e7a1d06c44c8c160045a866e90569ab Author: Dag-Erling Smørgrav AuthorDate: 2024-05-10 21:15:54 +0000 Commit: Dag-Erling Smørgrav CommitDate: 2024-05-10 21:16:26 +0000 tftpd: silence gcc overflow warnings GCC 13 complains that we might be writing too much to an on-stack buffer when createing a filename. In practice there is a check that filename isn't too long given the time format and other static characters so GCC is incorrect, but GCC isn't wrong that we're potentially trying to put a MAXPATHLEN length string + some other characters into a MAXPATHLEN buffer (if you ignore the check GCC can't realistically evaluate at compile time). Switch to snprintf to populate filename to ensure that future logic errors don't result in a stack overflow. Shorten the questionably named yyyymmdd buffer enough to slience the warning (checking the snprintf return value isn't sufficent) while preserving maximum flexibility for admins who use the -F option. MFC after: 3 days Sponsored by: Klara, Inc. Reviewed by: brooks Differential Revision: https://reviews.freebsd.org/D45086 --- libexec/tftpd/tftpd.c | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/libexec/tftpd/tftpd.c b/libexec/tftpd/tftpd.c index 80497738f60d..3f67ad2920cf 100644 --- a/libexec/tftpd/tftpd.c +++ b/libexec/tftpd/tftpd.c @@ -611,12 +611,20 @@ tftp_rrq(int peer, char *recvbuffer, size_t size) static int find_next_name(char *filename, int *fd) { - int i; + /* + * GCC "knows" that we might write all of yyyymmdd plus the static + * elemenents in the format into into newname and thus complains + * unless we reduce the size. This array is still too big, but since + * the format is user supplied, it's not clear what a better limit + * value would be and this is sufficent to silence the warnings. + */ + static const int suffix_len = strlen("..00"); + char yyyymmdd[MAXPATHLEN - suffix_len]; + char newname[MAXPATHLEN]; + int i, ret; time_t tval; - size_t len; + size_t len, namelen; struct tm lt; - char yyyymmdd[MAXPATHLEN]; - char newname[MAXPATHLEN]; /* Create the YYYYMMDD part of the filename */ time(&tval); @@ -624,26 +632,33 @@ find_next_name(char *filename, int *fd) len = strftime(yyyymmdd, sizeof(yyyymmdd), newfile_format, <); if (len == 0) { syslog(LOG_WARNING, - "Filename suffix too long (%d characters maximum)", - MAXPATHLEN); + "Filename suffix too long (%zu characters maximum)", + sizeof(yyyymmdd) - 1); return (EACCESS); } /* Make sure the new filename is not too long */ - if (strlen(filename) > MAXPATHLEN - len - 5) { + namelen = strlen(filename); + if (namelen >= sizeof(newname) - len - suffix_len) { syslog(LOG_WARNING, - "Filename too long (%zd characters, %zd maximum)", - strlen(filename), MAXPATHLEN - len - 5); + "Filename too long (%zu characters, %zu maximum)", + namelen, + sizeof(newname) - len - suffix_len - 1); return (EACCESS); } /* Find the first file which doesn't exist */ for (i = 0; i < 100; i++) { - sprintf(newname, "%s.%s.%02d", filename, yyyymmdd, i); - *fd = open(newname, - O_WRONLY | O_CREAT | O_EXCL, - S_IRUSR | S_IWUSR | S_IRGRP | - S_IWGRP | S_IROTH | S_IWOTH); + ret = snprintf(newname, sizeof(newname), "%s.%s.%02d", + filename, yyyymmdd, i); + /* + * Size checked above so this can't happen, we'd use a + * (void) cast, but gcc intentionally ignores that if + * snprintf has __attribute__((warn_unused_result)). + */ + if (ret < 0 || (size_t)ret >= sizeof(newname)) + __unreachable(); + *fd = open(newname, O_WRONLY | O_CREAT | O_EXCL, 0666); if (*fd > 0) return 0; }