Skip to content

Commit 23ef917

Browse files
authored
libpam migauthhandler buffers (#312)
A few corrections to prevent buffer overflow in the libpam migauthhandler
2 parents f698e24 + c3b6c99 commit 23ef917

File tree

1 file changed

+11
-5
lines changed

1 file changed

+11
-5
lines changed

mig/src/libpam-mig/migauthhandler.c

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,9 @@ static bool mig_reg_auth_attempt(const unsigned int mode,
368368
/* We don't exit hard here to make sure other auth types may follow */
369369
return false;
370370
}
371-
char pycmd[MAX_PYCMD_LENGTH] =
371+
/* NOTE: Allocate space for string terminator '\0' added by strncat after concatenation */
372+
char pycmd[MAX_PYCMD_LENGTH+1] =
372373
"(authorized, disconnect) = validate_auth_attempt(configuration, 'sftp-subsys', ";
373-
char pytmp[MAX_PYCMD_LENGTH];
374374
/* Always password auth here as mentioned in the above comment */
375375
strncat(&pycmd[0], "'password', ", MAX_PYCMD_LENGTH - strlen(pycmd));
376376

@@ -380,8 +380,9 @@ static bool mig_reg_auth_attempt(const unsigned int mode,
380380
strncat(&pycmd[0], address, MAX_PYCMD_LENGTH - strlen(pycmd));
381381
strncat(&pycmd[0], "', ", MAX_PYCMD_LENGTH - strlen(pycmd));
382382
if (secret != NULL) {
383-
sprintf(&pytmp[0], "secret='%s', ", secret);
384-
strncat(&pycmd[0], &pytmp[0], MAX_PYCMD_LENGTH - strlen(pycmd));
383+
strncat(&pycmd[0], "secret='", MAX_PYCMD_LENGTH - strlen(pycmd));
384+
strncat(&pycmd[0], secret, MAX_PYCMD_LENGTH - strlen(pycmd));
385+
strncat(&pycmd[0], "', ", MAX_PYCMD_LENGTH - strlen(pycmd));
385386
}
386387
if (mode & MIG_INVALID_USERNAME) {
387388
strncat(&pycmd[0], "invalid_username=True, ",
@@ -444,7 +445,12 @@ static bool mig_reg_auth_attempt(const unsigned int mode,
444445
MAX_PYCMD_LENGTH - strlen(pycmd));
445446
}
446447
strncat(&pycmd[0], ")", MAX_PYCMD_LENGTH - strlen(pycmd));
447-
/* Execute python command if and only if it didn't overflow */
448+
/* Execute python command if and only if it didn't overflow
449+
* NOTE: Since we can't check if pycmd was truncated by strncat
450+
* or if we actually got a command string length of MAX_PYCMD_LENGTH
451+
* we cap the maximum allowed command string length to MAX_PYCMD_LENGTH-1
452+
* (MAX_PYCMD_LENGTH including the terminator '\0')
453+
*/
448454
if (MAX_PYCMD_LENGTH > strlen(pycmd)) {
449455
pyrun(&pycmd[0]);
450456
PyObject *py_authorized = PyObject_GetAttrString(py_main, "authorized");

0 commit comments

Comments
 (0)