-
Notifications
You must be signed in to change notification settings - Fork 134
Description
PAM modules can send a num_msg, which is handled improperly by sudo-rs, as it is assumed this is a positive number without any checks.
Negative PAM message counts cause integer underflow and crash in sudo-rs (see src/pam/converse.rs lines 184 to 260 below). The conversation handler casts num_msg: c_int to usize without bounds checking; -1 becomes usize::MAX, so Vec::with_capacity(num_msg as usize) panics with a capacity overflow. That panic is caught, sets the panicked flag, and PamContext::authenticate immediately panic!s, killing sudo-rs. A single malicious PAM module (or one compromised via another bug) can therefore DOS sudo-rs, preventing ANY sudo-rs command from any user. Valid PAM modules never send a negative count, but a hostile, misconfigured (this is how I caught it) or compromised module can.
To Reproduce
Spooky C code ahead.
- Install or compile
sudo-rs(obvious)
# malicious PAM module that crashes sudo-rs
tee /tmp/pam_crash.c <<'EOF'
#define _GNU_SOURCE
#include <security/pam_modules.h>
#include <stddef.h>
PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags,
int argc, const char **argv) {
const struct pam_conv *conv;
if (pam_get_item(pamh, PAM_CONV, (const void **)&conv) == PAM_SUCCESS && conv && conv->conv) {
conv->conv(-1, NULL, NULL, conv->appdata_ptr); // send num_msg = -1
}
return PAM_SUCCESS;
}
PAM_EXTERN int pam_sm_setcred(pam_handle_t *pamh, int flags,
int argc, const char **argv) {
return PAM_SUCCESS;
}
EOF
# compile
gcc -fPIC -shared -o /tmp/pam_crash.so /tmp/pam_crash.c -lpam
# Adds "auth requisite /tmp/pam_crash.so" at the top of /etc/pam.d/sudo, basically gets the module into PAM flow
sudo-rs sed -i '1i\auth requisite /tmp/pam_crash.so' /etc/pam.d/sudo
Result: After any sudo-rs command, PAM panics after a successful login:
[alice@pc]$ sudo-rs su
thread 'main' panicked at src/pam/converse.rs:191:29:
capacity overflow
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[sudo: authenticate] Password:
thread 'main' panicked at src/pam/mod.rs:160:13:
Panic during pam authentication
Expected behavior
No crash, maybe a discard of num_msg or a warning.
Environment (please complete the following information):
- Linux distribution: Validated on Arch linux, should happen on all distros.
sudo-rscommit hash: bug working on v0.2.9, latest git tree commit 2b4d403. Didn't check when this was introduced.
Additional context
Code affected:
/src/pam/converse.rs lines 198 forward:
let result = std::panic::catch_unwind(|| {
let mut resp_bufs = Vec::with_capacity(num_msg as usize);
for i in 0..num_msg as usize {
// convert the input messages to Rust types
// SAFETY: the PAM contract ensures that `num_msg` does not exceed the amount
// of messages presented to this function in `msg`, and that it is not being
// written to at the same time as we are reading it. Note that the reference
// we create does not escape this loopy body.
let message: &pam_message = unsafe { &**msg.add(i) };
// SAFETY: PAM ensures that the messages passed are properly null-terminated
let msg = unsafe { string_from_ptr(message.msg) };
let style = if let Some(style) = PamMessageStyle::from_int(message.msg_style) {
style
} else {
// early return if there is a failure to convert, pam would have given us nonsense
return PamErrorType::ConversationError;
};
// send the conversation off to the Rust part
// SAFETY: appdata_ptr contains the `*mut ConverserData` that is untouched by PAM
let app_data = unsafe { &mut *(appdata_ptr as *mut ConverserData<C>) };
match handle_message(app_data, style, &msg) {
Ok(resp_buf) => {
resp_bufs.push(resp_buf);
}
Err(PamError::TimedOut) => {
app_data.timed_out = true;
return PamErrorType::ConversationError;
}
Err(_) => return PamErrorType::ConversationError,
}
}
// Allocate enough memory for the responses, which are initialized with zero.
// SAFETY: this will either allocate the required amount of (initialized) bytes,
// or return a null pointer.
let temp_resp = unsafe {
libc::calloc(
num_msg as libc::size_t,
std::mem::size_of::<pam_response>() as libc::size_t,
)
} as *mut pam_response;
if temp_resp.is_null() {
return PamErrorType::BufferError;
}
See my suggested patch : #1312