ldap: truly enforce max-tx

Ticket: 7465

If a bug chunk of data is parsed in one go, we could create many
transactions even if marking them as complete, and have
quadratic complexity calling find_request.

Proposed solution is to fail on creating a new transaction if too
many already exist.
pull/12358/head
Philippe Antoine 2 years ago committed by Victor Julien
parent 3b76c78cb9
commit 494d7bfe99

@ -89,7 +89,6 @@ pub struct LdapState {
state_data: AppLayerStateData, state_data: AppLayerStateData,
tx_id: u64, tx_id: u64,
transactions: VecDeque<LdapTransaction>, transactions: VecDeque<LdapTransaction>,
tx_index_completed: usize,
request_frame: Option<Frame>, request_frame: Option<Frame>,
response_frame: Option<Frame>, response_frame: Option<Frame>,
request_gap: bool, request_gap: bool,
@ -114,7 +113,6 @@ impl LdapState {
state_data: AppLayerStateData::new(), state_data: AppLayerStateData::new(),
tx_id: 0, tx_id: 0,
transactions: VecDeque::new(), transactions: VecDeque::new(),
tx_index_completed: 0,
request_frame: None, request_frame: None,
response_frame: None, response_frame: None,
request_gap: false, request_gap: false,
@ -138,7 +136,6 @@ impl LdapState {
} }
} }
if found { if found {
self.tx_index_completed = 0;
self.transactions.remove(index); self.transactions.remove(index);
} }
} }
@ -147,14 +144,9 @@ impl LdapState {
self.transactions.iter().find(|tx| tx.tx_id == tx_id + 1) self.transactions.iter().find(|tx| tx.tx_id == tx_id + 1)
} }
pub fn new_tx(&mut self) -> LdapTransaction { pub fn new_tx(&mut self) -> Option<LdapTransaction> {
let mut tx = LdapTransaction::new();
self.tx_id += 1;
tx.tx_id = self.tx_id;
if self.transactions.len() > unsafe { LDAP_MAX_TX } { if self.transactions.len() > unsafe { LDAP_MAX_TX } {
let mut index = self.tx_index_completed; for tx_old in &mut self.transactions {
for tx_old in &mut self.transactions.range_mut(self.tx_index_completed..) {
index += 1;
if !tx_old.complete { if !tx_old.complete {
tx_old.tx_data.updated_tc = true; tx_old.tx_data.updated_tc = true;
tx_old.tx_data.updated_ts = true; tx_old.tx_data.updated_ts = true;
@ -162,12 +154,14 @@ impl LdapState {
tx_old tx_old
.tx_data .tx_data
.set_event(LdapEvent::TooManyTransactions as u8); .set_event(LdapEvent::TooManyTransactions as u8);
break;
} }
} }
self.tx_index_completed = index; return None;
} }
return tx; let mut tx = LdapTransaction::new();
self.tx_id += 1;
tx.tx_id = self.tx_id;
return Some(tx);
} }
fn set_event(&mut self, e: LdapEvent) { fn set_event(&mut self, e: LdapEvent) {
@ -228,7 +222,11 @@ impl LdapState {
} }
match ldap_parse_msg(start) { match ldap_parse_msg(start) {
Ok((rem, msg)) => { Ok((rem, msg)) => {
let mut tx = self.new_tx(); let tx = self.new_tx();
if tx.is_none() {
return AppLayerResult::err();
}
let mut tx = tx.unwrap();
let tx_id = tx.id(); let tx_id = tx.id();
let request = LdapMessage::from(msg); let request = LdapMessage::from(msg);
// check if STARTTLS was requested // check if STARTTLS was requested
@ -237,7 +235,7 @@ impl LdapState {
self.request_tls = true; self.request_tls = true;
} }
} }
tx.complete = tx_is_complete(&request.protocol_op, Direction::ToServer); tx.complete |= tx_is_complete(&request.protocol_op, Direction::ToServer);
tx.request = Some(request); tx.request = Some(request);
self.transactions.push_back(tx); self.transactions.push_back(tx);
let consumed = start.len() - rem.len(); let consumed = start.len() - rem.len();
@ -298,8 +296,7 @@ impl LdapState {
let response = LdapMessage::from(msg); let response = LdapMessage::from(msg);
// check if STARTTLS was requested // check if STARTTLS was requested
if self.request_tls { if self.request_tls {
if let ProtocolOp::ExtendedResponse(response) = &response.protocol_op if let ProtocolOp::ExtendedResponse(response) = &response.protocol_op {
{
if response.result.result_code == ResultCode(0) { if response.result.result_code == ResultCode(0) {
SCLogDebug!("LDAP: STARTTLS detected"); SCLogDebug!("LDAP: STARTTLS detected");
self.has_starttls = true; self.has_starttls = true;
@ -308,7 +305,7 @@ impl LdapState {
} }
} }
if let Some(tx) = self.find_request(response.message_id) { if let Some(tx) = self.find_request(response.message_id) {
tx.complete = tx_is_complete(&response.protocol_op, Direction::ToClient); tx.complete |= tx_is_complete(&response.protocol_op, Direction::ToClient);
let tx_id = tx.id(); let tx_id = tx.id();
tx.tx_data.updated_tc = true; tx.tx_data.updated_tc = true;
tx.responses.push_back(response); tx.responses.push_back(response);
@ -317,7 +314,11 @@ impl LdapState {
} else if let ProtocolOp::ExtendedResponse(_) = response.protocol_op { } else if let ProtocolOp::ExtendedResponse(_) = response.protocol_op {
// this is an unsolicited notification, which means // this is an unsolicited notification, which means
// there is no request // there is no request
let mut tx = self.new_tx(); let tx = self.new_tx();
if tx.is_none() {
return AppLayerResult::err();
}
let mut tx = tx.unwrap();
let tx_id = tx.id(); let tx_id = tx.id();
tx.complete = true; tx.complete = true;
tx.responses.push_back(response); tx.responses.push_back(response);
@ -325,7 +326,11 @@ impl LdapState {
let consumed = start.len() - rem.len(); let consumed = start.len() - rem.len();
self.set_frame_tc(flow, tx_id, consumed as i64); self.set_frame_tc(flow, tx_id, consumed as i64);
} else { } else {
let mut tx = self.new_tx(); let tx = self.new_tx();
if tx.is_none() {
return AppLayerResult::err();
}
let mut tx = tx.unwrap();
tx.complete = true; tx.complete = true;
let tx_id = tx.id(); let tx_id = tx.id();
tx.responses.push_back(response); tx.responses.push_back(response);
@ -367,9 +372,13 @@ impl LdapState {
match ldap_parse_msg(input) { match ldap_parse_msg(input) {
Ok((_, msg)) => { Ok((_, msg)) => {
let mut tx = self.new_tx(); let tx = self.new_tx();
if tx.is_none() {
return AppLayerResult::err();
}
let mut tx = tx.unwrap();
let request = LdapMessage::from(msg); let request = LdapMessage::from(msg);
tx.complete = tx_is_complete(&request.protocol_op, Direction::ToServer); tx.complete |= tx_is_complete(&request.protocol_op, Direction::ToServer);
tx.request = Some(request); tx.request = Some(request);
self.transactions.push_back(tx); self.transactions.push_back(tx);
} }
@ -411,7 +420,7 @@ impl LdapState {
Ok((rem, msg)) => { Ok((rem, msg)) => {
let response = LdapMessage::from(msg); let response = LdapMessage::from(msg);
if let Some(tx) = self.find_request(response.message_id) { if let Some(tx) = self.find_request(response.message_id) {
tx.complete = tx_is_complete(&response.protocol_op, Direction::ToClient); tx.complete |= tx_is_complete(&response.protocol_op, Direction::ToClient);
let tx_id = tx.id(); let tx_id = tx.id();
tx.responses.push_back(response); tx.responses.push_back(response);
let consumed = start.len() - rem.len(); let consumed = start.len() - rem.len();
@ -419,7 +428,11 @@ impl LdapState {
} else if let ProtocolOp::ExtendedResponse(_) = response.protocol_op { } else if let ProtocolOp::ExtendedResponse(_) = response.protocol_op {
// this is an unsolicited notification, which means // this is an unsolicited notification, which means
// there is no request // there is no request
let mut tx = self.new_tx(); let tx = self.new_tx();
if tx.is_none() {
return AppLayerResult::err();
}
let mut tx = tx.unwrap();
tx.complete = true; tx.complete = true;
let tx_id = tx.id(); let tx_id = tx.id();
tx.responses.push_back(response); tx.responses.push_back(response);
@ -427,7 +440,11 @@ impl LdapState {
let consumed = start.len() - rem.len(); let consumed = start.len() - rem.len();
self.set_frame_tc(flow, tx_id, consumed as i64); self.set_frame_tc(flow, tx_id, consumed as i64);
} else { } else {
let mut tx = self.new_tx(); let tx = self.new_tx();
if tx.is_none() {
return AppLayerResult::err();
}
let mut tx = tx.unwrap();
tx.complete = true; tx.complete = true;
let tx_id = tx.id(); let tx_id = tx.id();
tx.responses.push_back(response); tx.responses.push_back(response);

Loading…
Cancel
Save