mir/sudo
2
0
Fork 0
mirror of https://github.com/sudo-project/sudo.git synced 2025-08-22 09:57:41 +00:00
Code Issues Releases Wiki Activity

Check for negative return value of read, write and lseek instead of -1

Browse Source 
The return values are used in ways that assume they are positive.
In practice, it is not possible to have a negative return value
other than -1 due to the size of the buffers being read from or
written to.  Also add overflow checks when updating the buffer len.
Quiets several coverity warnings.
This commit is contained in:
Todd C. Miller 2025-01-14 14:07:52 -07:00
parent 6df96785ff
commit a27b989c9c
22 changed files with 172 additions and 124 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/fuzzstub/fuzzstub.c
View File

@ -104,7 +104,7 @@ main(int argc, char *argv[])
}
nread = read(fd, buf, filesize);
if ((size_t)nread != filesize) {
if (nread == -1)
if (nread < 0)
fprintf(stderr, "read %s: %s\n", arg, strerror(errno));
else
fprintf(stderr, "read %s: short read\n", arg);

2
lib/iolog/iolog_nextid.c
View File

@ -107,7 +107,7 @@ iolog_nextid(const char *iolog_dir, char sessid[7])
/* Read current seq number (base 36). */
nread = read(fd, buf, sizeof(buf) - 1);
if (nread != 0) {
if (nread == -1) {
if (nread < 0) {
goto done;
}
if (buf[nread - 1] == '\n')

6
lib/iolog/regress/iolog_filter/check_iolog_filter.c
View File

@ -138,7 +138,7 @@ main(int argc, char *argv[])
nread = read(fd, tbuf, timing.u.nbytes);
if ((size_t)nread != timing.u.nbytes) {
if (nread == -1)
if (nread < 0)
sudo_warn("%s/%s", argv[i], name);
else
sudo_warnx("%s/%s: short read", argv[i], name);
@ -155,8 +155,8 @@ main(int argc, char *argv[])
if (timing.event == IO_EVENT_TTYIN) {
nread = read(ttyin_ok_fd, fbuf, timing.u.nbytes);
if (nread == -1) {
if (nread == -1)
if ((size_t)nread != timing.u.nbytes) {
if (nread < 0)
sudo_warn("%s/ttyin.filtered", argv[i]);
else
sudo_warnx("%s/ttyin.filtered: short read", argv[i]);

2
lib/util/event.c
View File

@ -154,7 +154,7 @@ signal_pipe_cb(int fd, int what, void *v)
sudo_debug_printf(SUDO_DEBUG_INFO,
"%s: received signal %d", __func__, (int)ch);
}
if (nread == -1 && errno != EAGAIN) {
if (nread < 0 && errno != EAGAIN) {
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO,
"%s: error reading from signal pipe fd %d", __func__, fd);
}

2
lib/util/regress/fuzz/fuzz_sudo_conf.c
View File

@ -85,7 +85,7 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
if (fd == -1)
return 0;
nwritten = write(fd, data, size);
if (nwritten == -1) {
if ((size_t)nwritten != size) {
close(fd);
return 0;
}

39
logsrvd/logsrvd.c
View File

@ -969,13 +969,17 @@ server_msg_cb(int fd, int what, void *v)
} else
#endif
{
nwritten = (size_t)write(fd, buf->data + buf->off, buf->len - buf->off);
const ssize_t n = write(fd, buf->data + buf->off, buf->len - buf->off);
if (n < 0) {
if (errno == EAGAIN || errno == EINTR)
debug_return;
sudo_warn("%s: write", closure->ipaddr);
goto finished;
}
nwritten = (size_t)n;
}
if (nwritten == (size_t)-1) {
if (errno == EAGAIN || errno == EINTR)
debug_return;
sudo_warn("%s: write", closure->ipaddr);
if (nwritten > SIZE_MAX - buf->off) {
sudo_warnx(U_("internal error, %s overflow"), __func__);
goto finished;
}
buf->off += nwritten;
@ -1082,25 +1086,28 @@ client_msg_cb(int fd, int what, void *v)
} else
#endif
{
nread = (size_t)read(fd, buf->data + buf->len, buf->size - buf->len);
const ssize_t n = read(fd, buf->data + buf->len, buf->size - buf->len);
if (n < 0) {
if (errno == EAGAIN || errno == EINTR)
debug_return;
sudo_warn("%s: read", closure->ipaddr);
goto close_connection;
}
nread = (size_t)n;
}
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: received %zd bytes from client %s",
__func__, nread, closure->ipaddr);
switch (nread) {
case (size_t)-1:
if (errno == EAGAIN || errno == EINTR)
debug_return;
sudo_warn("%s: read", closure->ipaddr);
goto close_connection;
case 0:
if (nread == 0) {
if (closure->state != FINISHED) {
sudo_debug_printf(SUDO_DEBUG_WARN|SUDO_DEBUG_LINENO,
"unexpected EOF");
}
goto close_connection;
default:
break;
}
if (nread > SIZE_MAX - buf->len) {
sudo_warnx(U_("internal error, %s overflow"), __func__);
goto close_connection;
}
buf->len += nread;

2
logsrvd/logsrvd_local.c
View File

@ -376,7 +376,7 @@ store_exit_info_json(int dfd, struct eventlog *evlog)
iov[1].iov_len = sudo_json_get_len(&jsonc);
iov[2].iov_base = (char *)"\n}\n";
iov[2].iov_len = 3;
if (writev(fd, iov, 3) == -1) {
if (writev(fd, iov, 3) < 0) {
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO,
"unable to write %s/log.json", evlog->iolog_path);
/* Back up and try to restore to original state. */

38
logsrvd/logsrvd_relay.c
View File

@ -798,23 +798,26 @@ relay_server_msg_cb(int fd, int what, void *v)
} else
#endif
{
ssize_t n;
sudo_debug_printf(SUDO_DEBUG_INFO,
"%s: ServerMessage from relay %s (%s)", __func__,
relay_closure->relay_name.name, relay_closure->relay_name.ipaddr);
nread = (size_t)read(fd, buf->data + buf->len, buf->size - buf->len);
n = read(fd, buf->data + buf->len, buf->size - buf->len);
if (n < 0) {
if (errno == EAGAIN || errno == EINTR)
debug_return;
sudo_warn("%s: read", relay_closure->relay_name.ipaddr);
closure->errstr = _("error reading from relay");
goto send_error;
}
nread = (size_t)n;
}
sudo_debug_printf(SUDO_DEBUG_INFO,
"%s: received %zd bytes from relay %s (%s)", __func__, nread,
relay_closure->relay_name.name, relay_closure->relay_name.ipaddr);
switch (nread) {
case (size_t)-1:
if (errno == EAGAIN || errno == EINTR)
debug_return;
sudo_warn("%s: read", relay_closure->relay_name.ipaddr);
closure->errstr = _("unable to read from relay");
goto send_error;
case 0:
if (nread == 0) {
/* EOF from relay server, close the socket. */
shutdown(relay_closure->sock, SHUT_RDWR);
close(relay_closure->sock);
@ -833,8 +836,11 @@ relay_server_msg_cb(int fd, int what, void *v)
if (closure->sock == -1)
connection_close(closure);
debug_return;
default:
break;
}
if (nread > SIZE_MAX - buf->len) {
sudo_warnx(U_("internal error, %s overflow"), __func__);
closure->errstr = _("error reading from relay");
goto send_error;
}
buf->len += nread;
@ -979,14 +985,20 @@ relay_client_msg_cb(int fd, int what, void *v)
} else
#endif
{
nwritten = (size_t)write(fd, buf->data + buf->off, buf->len - buf->off);
if (nwritten == (size_t)-1) {
const ssize_t n = write(fd, buf->data + buf->off, buf->len - buf->off);
if (n < 0) {
if (errno == EAGAIN || errno == EINTR)
debug_return;
sudo_warn("%s: write", relay_closure->relay_name.ipaddr);
closure->errstr = _("error writing to relay");
goto send_error;
}
nwritten = (size_t)n;
}
if (nwritten > SIZE_MAX - buf->off) {
sudo_warnx(U_("internal error, %s overflow"), __func__);
closure->errstr = _("error writing to relay");
goto send_error;
}
buf->off += nwritten;

2
logsrvd/regress/fuzz/fuzz_logsrvd_conf.c
View File

@ -193,7 +193,7 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
if (fd == -1)
return 0;
nwritten = write(fd, data, size);
if (nwritten == -1) {
if ((size_t)nwritten != size) {
close(fd);
return 0;
}

40
logsrvd/sendlog.c
View File

@ -1370,23 +1370,28 @@ server_msg_cb(int fd, int what, void *v)
} else
#endif
{
ssize_t n;
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: reading ServerMessage", __func__);
nread = (size_t)read(fd, buf->data + buf->len, buf->size - buf->len);
n = read(fd, buf->data + buf->len, buf->size - buf->len);
if (n < 0) {
if (errno == EAGAIN || errno == EINTR)
debug_return;
sudo_warn("read");
goto bad;
}
nread = (size_t)n;
}
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: received %zd bytes from server",
__func__, nread);
switch (nread) {
case (size_t)-1:
if (errno == EAGAIN || errno == EINTR)
debug_return;
sudo_warn("read");
goto bad;
case 0:
if (nread == 0) {
if (closure->state != FINISHED)
sudo_warnx("%s", U_("premature EOF"));
goto bad;
default:
break;
}
if (nread > SIZE_MAX - buf->len) {
sudo_warnx(U_("internal error, %s overflow"), __func__);
goto bad;
}
buf->len += nread;
@ -1496,12 +1501,17 @@ client_msg_cb(int fd, int what, void *v)
} else
#endif
{
nwritten = (size_t)write(fd, buf->data + buf->off, buf->len - buf->off);
const ssize_t n = write(fd, buf->data + buf->off, buf->len - buf->off);
if (n < 0) {
if (errno == EAGAIN || errno == EINTR)
debug_return;
sudo_warn("write");
goto bad;
}
nwritten = (size_t)n;
}
if (nwritten == (size_t)-1) {
if (errno == EAGAIN || errno == EINTR)
debug_return;
sudo_warn("write");
if (nwritten > SIZE_MAX - buf->off) {
sudo_warnx(U_("internal error, %s overflow"), __func__);
goto bad;
}
buf->off += nwritten;

16
plugins/sudoers/ldap.c
View File

@ -1293,21 +1293,27 @@ sudo_krb5_copy_cc_file(struct sudoers_context *ctx)
do {
nwritten = write(nfd, buf + off,
(size_t)(nread - off));
if (nwritten == -1) {
if (nwritten < 0) {
sudo_warn("error writing to %s", new_ccname);
goto write_error;
}
if (nwritten > SSIZE_MAX - off) {
sudo_warnx(U_("internal error, %s overflow"),
__func__);
nwritten = -1;
goto write_error;
}
off += nwritten;
} while (off < nread);
}
if (nread == -1)
if (nread < 0)
sudo_warn("unable to read %s", new_ccname);
write_error:
close(nfd);
if (nread != -1 && nwritten != -1) {
ret = new_ccname; /* success! */
} else {
if (nread < 0 || nwritten < 0) {
unlink(new_ccname); /* failed */
} else {
ret = new_ccname; /* success! */
}
} else {
sudo_warn("unable to create temp file %s", new_ccname);

35
plugins/sudoers/log_client.c
View File

@ -1791,21 +1791,24 @@ server_msg_cb(int fd, int what, void *v)
} else
#endif /* HAVE_OPENSSL */
{
nread = (size_t)read(fd, buf->data + buf->len, buf->size - buf->len);
const ssize_t n = read(fd, buf->data + buf->len, buf->size - buf->len);
if (n < 0) {
if (errno == EAGAIN)
debug_return;
sudo_warn("read");
goto bad;
}
nread = (size_t)n;
}
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: received %zd bytes from server",
__func__, nread);
switch (nread) {
case (size_t)-1:
if (errno == EAGAIN)
debug_return;
sudo_warn("read");
goto bad;
case 0:
if (nread == 0) {
sudo_warnx("%s", U_("lost connection to log server"));
goto bad;
default:
break;
}
if (nread > SIZE_MAX - buf->len) {
sudo_warnx(U_("internal error, %s overflow"), __func__);
goto bad;
}
buf->len += nread;
@ -1928,11 +1931,15 @@ client_msg_cb(int fd, int what, void *v)
} else
#endif /* HAVE_OPENSSL */
{
nwritten = (size_t)write(fd, buf->data + buf->off, buf->len - buf->off);
const ssize_t n = write(fd, buf->data + buf->off, buf->len - buf->off);
if (n < 0) {
sudo_warn("send");
goto bad;
}
nwritten = (size_t)n;
}
if (nwritten == (size_t)-1) {
sudo_warn("send");
if (nwritten > SIZE_MAX - buf->off) {
sudo_warnx(U_("internal error, %s overflow"), __func__);
goto bad;
}
buf->off += nwritten;

2
plugins/sudoers/timestamp.c
View File

@ -348,7 +348,7 @@ ts_write(const struct sudoers_context *ctx, int fd, const char *fname,
nwritten = pwrite(fd, entry, entry->size, offset);
}
if ((size_t)nwritten != entry->size) {
if (nwritten == -1) {
if (nwritten < 0) {
log_warning(ctx, SLOG_SEND_MAIL,
N_("unable to write to %s"), fname);
} else {

2
plugins/sudoers/tsdump.c
View File

@ -147,7 +147,7 @@ main(int argc, char *argv[])
if ((nread = read(fd, &cur, sizeof(cur))) == 0)
break;
if (nread == -1)
if (nread < 0)
sudo_fatal(U_("unable to read %s"), fname);
valid = valid_entry(&cur, pos);

4
plugins/sudoers/visudo.c
View File

@ -510,7 +510,7 @@ edit_sudoers(struct sudoersfile *sp, char *editor, int editor_argc,
(void) lseek(sp->fd, (off_t)0, SEEK_SET);
while ((nread = read(sp->fd, buf, sizeof(buf))) > 0) {
if (write(tfd, buf, (size_t)nread) == -1)
if (write(tfd, buf, (size_t)nread) != nread)
sudo_fatal("%s", U_("write error"));
lastch = buf[nread - 1];
}
@ -518,7 +518,7 @@ edit_sudoers(struct sudoersfile *sp, char *editor, int editor_argc,
/* Add missing newline at EOF if needed. */
if (lastch != '\n') {
lastch = '\n';
if (write(tfd, &lastch, 1) == -1)
if (write(tfd, &lastch, 1) != 1)
sudo_fatal("%s", U_("write error"));
}
}

4
src/conversation.c
View File

@ -113,8 +113,8 @@ sudo_conversation(int num_msgs, const struct sudo_conv_message msgs[],
}
if (ttyfd != -1) {
/* Try writing to tty but fall back to fp on error. */
if ((len == 0 || write(ttyfd, msg->msg, len) != -1) &&
(crnl == NULL || write(ttyfd, crnl, 2) != -1)) {
if ((len == 0 || write(ttyfd, msg->msg, len) > 0) &&
(crnl == NULL || write(ttyfd, crnl, 2) > 0)) {
written = true;
}
close(ttyfd);

49
src/copy_file.c
View File

@ -43,38 +43,42 @@ sudo_extend_file(int fd, const char *name, off_t new_size)
{
off_t old_size, size;
ssize_t nwritten;
int serrno;
char zeroes[BUFSIZ] = { '\0' };
debug_decl(sudo_extend_file, SUDO_DEBUG_UTIL);
if ((old_size = lseek(fd, 0, SEEK_END)) == -1) {
if ((old_size = lseek(fd, 0, SEEK_END)) < 0) {
sudo_warn("lseek");
debug_return_int(-1);
}
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: extending %s from %lld to %lld",
__func__, name, (long long)old_size, (long long)new_size);
for (size = old_size; size < new_size; size += nwritten) {
for (size = old_size; size < new_size; ) {
off_t len = new_size - size;
if (len > ssizeof(zeroes))
len = ssizeof(zeroes);
nwritten = write(fd, zeroes, (size_t)len);
if (nwritten == -1) {
int serrno = errno;
if (ftruncate(fd, old_size) == -1) {
sudo_debug_printf(
SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO,
"unable to truncate %s to %lld", name, (long long)old_size);
}
errno = serrno;
debug_return_int(-1);
if (nwritten < 0 || nwritten > OFF_T_MAX - size) {
goto fail;
}
size += nwritten;
}
if (lseek(fd, 0, SEEK_SET) == -1) {
if (lseek(fd, 0, SEEK_SET) < 0) {
sudo_warn("lseek");
debug_return_int(-1);
}
debug_return_int(0);
fail:
serrno = errno;
if (ftruncate(fd, old_size) != 0) {
sudo_debug_printf(
SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO,
"unable to truncate %s to %lld", name, (long long)old_size);
}
errno = serrno;
debug_return_int(-1);
}
/*
@ -107,24 +111,27 @@ sudo_copy_file(const char *src, int src_fd, off_t src_len, const char *dst,
}
/* Overwrite the old file with the new contents. */
while ((nread = read(src_fd, buf, sizeof(buf))) > 0) {
for (;;) {
ssize_t off = 0;
do {
nread = read(src_fd, buf, sizeof(buf));
if (nread <= 0) {
if (nread == 0)
break;
sudo_warn(U_("unable to read from %s"), src);
debug_return_int(-1);
}
while (nread > off) {
nwritten = write(dst_fd, buf + off, (size_t)(nread - off));
if (nwritten == -1)
if (nwritten < 0 || nwritten > SSIZE_MAX - off)
goto write_error;
off += nwritten;
} while (nread > off);
}
if (nread == -1) {
sudo_warn(U_("unable to read from %s"), src);
debug_return_int(-1);
}
}
/* Did the file shrink? */
if (src_len < dst_len) {
/* We don't open with O_TRUNC so must truncate manually. */
if (ftruncate(dst_fd, src_len) == -1) {
if (ftruncate(dst_fd, src_len) != 0) {
sudo_debug_printf(
SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO,
"unable to truncate %s to %lld", dst, (long long)src_len);

33
src/exec_monitor.c
View File

@ -279,8 +279,7 @@ mon_errsock_cb(int fd, int what, void *v)
* Note that the error socket is *blocking*.
*/
nread = read(fd, &errval, sizeof(errval));
switch (nread) {
case -1:
if (nread < 0) {
if (errno != EAGAIN && errno != EINTR) {
if (mc->cstat->val == CMD_INVALID) {
/* XXX - need a way to distinguish non-exec error. */
@ -291,22 +290,22 @@ mon_errsock_cb(int fd, int what, void *v)
"%s: failed to read error socket", __func__);
sudo_ev_loopbreak(mc->evbase);
}
break;
default:
if (nread == 0) {
/* The error socket closes when the command is executed. */
sudo_debug_printf(SUDO_DEBUG_INFO, "EOF on error socket");
} else {
/* Errno value when child is unable to execute command. */
sudo_debug_printf(SUDO_DEBUG_INFO, "errno from child: %s",
strerror(errval));
mc->cstat->type = CMD_ERRNO;
mc->cstat->val = errval;
}
sudo_ev_del(mc->evbase, mc->errsock_event);
close(fd);
break;
debug_return;
}
if (nread == 0) {
/* The error socket closes when the command is executed. */
sudo_debug_printf(SUDO_DEBUG_INFO, "EOF on error socket");
} else {
/* Errno value when child is unable to execute command. */
sudo_debug_printf(SUDO_DEBUG_INFO, "errno from child: %s",
strerror(errval));
mc->cstat->type = CMD_ERRNO;
mc->cstat->val = errval;
}
sudo_ev_del(mc->evbase, mc->errsock_event);
close(fd);
debug_return;
}

2
src/exec_ptrace.c
View File

@ -1443,7 +1443,7 @@ proc_read_vec(pid_t pid, const char *name, int *countp, char ***vecp,
/* Read in strings until EOF. */
do {
nread = read(fd, strtab, remainder);
if (nread == -1) {
if (nread < 0) {
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
"%s: unable to read %s", __func__, path);
close(fd);

2
src/sudo_intercept_common.c
View File

@ -80,7 +80,7 @@ send_req(int sock, const void *buf, size_t len)
do {
nwritten = send(sock, cp, len, 0);
if (nwritten == -1) {
if (nwritten < 0) {
if (errno == EINTR)
continue;
debug_return_bool(false);

10
src/tgetpass.c
View File

@ -218,11 +218,11 @@ restart:
if (ISSET(flags, TGP_BELL) && output != STDERR_FILENO) {
/* Ring the bell if requested and there is a tty. */
if (write(output, "\a", 1) == -1)
if (write(output, "\a", 1) != 1)
goto restore;
}
if (prompt) {
if (write(output, prompt, strlen(prompt)) == -1)
if (write(output, prompt, strlen(prompt)) < 0)
goto restore;
}
@ -233,7 +233,7 @@ restart:
save_errno = errno;
if (neednl || pass == NULL) {
if (write(output, "\n", 1) == -1)
if (write(output, "\n", 1) != 1)
goto restore;
}
tgetpass_display_error(errval);
@ -403,7 +403,7 @@ getln(int fd, char *buf, size_t bufsiz, bool feedback,
break;
} else if (c == sudo_term_kill) {
while (cp > buf) {
if (write(fd, "\b \b", 3) == -1)
if (write(fd, "\b \b", 3) != 3)
break;
cp--;
}
@ -426,7 +426,7 @@ getln(int fd, char *buf, size_t bufsiz, bool feedback,
if (feedback) {
/* erase stars */
while (cp > buf) {
if (write(fd, "\b \b", 3) == -1)
if (write(fd, "\b \b", 3) != 3)
break;
--cp;
}

2
src/ttyname.c
View File

@ -248,7 +248,7 @@ get_process_ttyname(char *name, size_t namelen)
if ((fd = open(path, O_RDONLY | O_NOFOLLOW)) != -1) {
cp = buf;
while ((nread = read(fd, cp, sizeof(buf) - (size_t)(cp - buf))) != 0) {
if (nread == -1) {
if (nread < 0) {
if (errno == EAGAIN || errno == EINTR)
continue;
break;