mirror of
https://github.com/freebsd/freebsd-src
synced 2024-10-02 22:54:52 +00:00
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
This commit is contained in:
parent
4d09eb87c5
commit
25945af47e
|
@ -611,12 +611,20 @@ tftp_rrq(int peer, char *recvbuffer, size_t size)
|
|||
static int
|
||||
find_next_name(char *filename, int *fd)
|
||||
{
|
||||
int i;
|
||||
time_t tval;
|
||||
size_t len;
|
||||
struct tm lt;
|
||||
char yyyymmdd[MAXPATHLEN];
|
||||
/*
|
||||
* 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, namelen;
|
||||
struct tm lt;
|
||||
|
||||
/* 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;
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue