From 9555e3add65ecea6287c1218d9073d06513da34c Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 2 Jul 2026 20:36:08 +0200 Subject: [PATCH] rdp: fix tx id handling Tx ID handling did not take the required + 1 into account. From a report: RDP can skip cleanup because its id convention does not match the generic Rust iterator. The generic iterator in applayer.rs returns tx.id() - 1, and cleanup trusts that id when calling StateTransactionFree in app-layer-parser.c. RDP registers that iterator in rdp.rs, but RdpTransaction::id() returns the stored id unchanged in rdp.rs, while free_tx also compares against the raw stored id in rdp.rs. For a single freeable RDP tx with stored id 1, the iterator returns C id 0; cleanup calls free_tx(0), nothing is removed, then has_next == false allows min_id to advance to total_txs in app-layer-parser.c. That leaves the tx live but now below min_id, so later cleanup will not revisit it. This patch brings the handling in line with the other parsers. Bug: #8717. --- rust/src/rdp/rdp.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rust/src/rdp/rdp.rs b/rust/src/rdp/rdp.rs index 77bc82fbf2..f0faf52371 100644 --- a/rust/src/rdp/rdp.rs +++ b/rust/src/rdp/rdp.rs @@ -146,7 +146,7 @@ impl RdpState { let mut index = 0; for ii in 0..len { let tx = &self.transactions[ii]; - if tx.id == tx_id { + if tx.id == tx_id + 1 { found = true; index = ii; break; @@ -158,7 +158,7 @@ impl RdpState { } fn get_tx(&self, tx_id: u64) -> Option<&RdpTransaction> { - self.transactions.iter().find(|&tx| tx.id == tx_id) + self.transactions.iter().find(|&tx| tx.id == tx_id + 1) } fn new_tx(&mut self, item: RdpTransactionItem, direction: Direction) -> RdpTransaction { @@ -721,7 +721,7 @@ mod tests { state.transactions.push_back(tx0); state.transactions.push_back(tx1); state.transactions.push_back(tx2); - assert_eq!(Some(&state.transactions[1]), state.get_tx(2)); + assert_eq!(Some(&state.transactions[1]), state.get_tx(1)); } #[test] @@ -742,11 +742,11 @@ mod tests { state.transactions.push_back(tx0); state.transactions.push_back(tx1); state.transactions.push_back(tx2); - state.free_tx(1); + state.free_tx(0); assert_eq!(3, state.next_id); assert_eq!(2, state.transactions.len()); assert_eq!(2, state.transactions[0].id); assert_eq!(3, state.transactions[1].id); - assert_eq!(None, state.get_tx(1)); + assert_eq!(None, state.get_tx(0)); } }