portable-specific int overflow defence-in-depth

These too are unreachable, but we want the code to be safe regardless of
context. Reported by Yair Mizrahi @ JFrog
This commit is contained in:
Damien Miller 2023-07-14 15:34:47 +10:00
parent 2ee48adb9f
commit 4b94d09542
No known key found for this signature in database
2 changed files with 16 additions and 18 deletions

View File

@ -848,7 +848,7 @@ sshpam_query(void *ctx, char **name, char **info,
size_t plen; size_t plen;
u_char type; u_char type;
char *msg; char *msg;
size_t len, mlen; size_t len, mlen, nmsg = 0;
int r; int r;
debug3("PAM: %s entering", __func__); debug3("PAM: %s entering", __func__);
@ -861,6 +861,8 @@ sshpam_query(void *ctx, char **name, char **info,
plen = 0; plen = 0;
*echo_on = xmalloc(sizeof(u_int)); *echo_on = xmalloc(sizeof(u_int));
while (ssh_msg_recv(ctxt->pam_psock, buffer) == 0) { while (ssh_msg_recv(ctxt->pam_psock, buffer) == 0) {
if (++nmesg > PAM_MAX_NUM_MSG)
fatal_f("too many query messages");
if ((r = sshbuf_get_u8(buffer, &type)) != 0 || if ((r = sshbuf_get_u8(buffer, &type)) != 0 ||
(r = sshbuf_get_cstring(buffer, &msg, &mlen)) != 0) (r = sshbuf_get_cstring(buffer, &msg, &mlen)) != 0)
fatal("%s: buffer error: %s", __func__, ssh_err(r)); fatal("%s: buffer error: %s", __func__, ssh_err(r));

View File

@ -178,20 +178,20 @@ ssh_selinux_setup_pty(char *pwname, const char *tty)
void void
ssh_selinux_change_context(const char *newname) ssh_selinux_change_context(const char *newname)
{ {
int len, newlen; char *oldctx, *newctx, *cx, *cx2;
char *oldctx, *newctx, *cx;
LogLevel log_level = SYSLOG_LEVEL_INFO; LogLevel log_level = SYSLOG_LEVEL_INFO;
if (!ssh_selinux_enabled()) if (!ssh_selinux_enabled())
return; return;
if (getcon(&oldctx) < 0) { if (getcon(&oldctx) < 0) {
logit("%s: getcon failed with %s", __func__, strerror(errno)); logit_f("getcon failed with %s", strerror(errno));
return; return;
} }
if ((cx = index(oldctx, ':')) == NULL || (cx = index(cx + 1, ':')) == if ((cx = strchr(oldctx, ':')) == NULL ||
NULL) { (cx = strchr(cx + 1, ':')) == NULL ||
logit("%s: unparsable context %s", __func__, oldctx); (cx - oldctx) >= INT_MAX) {
logit_f("unparsable context %s", oldctx);
return; return;
} }
@ -203,18 +203,14 @@ ssh_selinux_change_context(const char *newname)
sizeof(SSH_SELINUX_UNCONFINED_TYPE) - 1) == 0) sizeof(SSH_SELINUX_UNCONFINED_TYPE) - 1) == 0)
log_level = SYSLOG_LEVEL_DEBUG3; log_level = SYSLOG_LEVEL_DEBUG3;
newlen = strlen(oldctx) + strlen(newname) + 1; cx2 = strchr(cx + 1, ':');
newctx = xmalloc(newlen); xasprintf(&newctx, "%.*s%s%s", (int)(cx - oldctx + 1), oldctx,
len = cx - oldctx + 1; newname, cx2 == NULL ? "" : cx2);
memcpy(newctx, oldctx, len);
strlcpy(newctx + len, newname, newlen - len); debug3_f("setting context from '%s' to '%s'", oldctx, newctx);
if ((cx = index(cx + 1, ':')))
strlcat(newctx, cx, newlen);
debug3("%s: setting context from '%s' to '%s'", __func__,
oldctx, newctx);
if (setcon(newctx) < 0) if (setcon(newctx) < 0)
do_log2(log_level, "%s: setcon %s from %s failed with %s", do_log2_f(log_level, "setcon %s from %s failed with %s",
__func__, newctx, oldctx, strerror(errno)); newctx, oldctx, strerror(errno));
free(oldctx); free(oldctx);
free(newctx); free(newctx);
} }