next/1478/20260904/v1

doc/userguide/configuration/suricata-yaml.rst

@@ -2015,6 +2015,19 @@ default is 1 MB.
       mqtt:
         max-msg-length: 1mb
 
+RFB
+~~~
+
+RFB can have some strings whose maximum length according to the RFC is 4GiB.
+In order to limit ram consumption and log output, a configuration parameter ``max-string-length`` is available.
+This limit will also apply during detection.
+An event ``rfb.too_long_string`` will be emitted when a string exceeds the limit. The default is 4 KiB.
+
+::
+
+      rfb:
+        max-string-length: 4 KiB
+
 SMTP
 ~~~~~~
 

doc/userguide/firewall/firewall-design.rst

@@ -122,6 +122,14 @@ alert
 action in firewall rules. The effect will be the creation of an alert event when the
 firewall rule matches.
 
+config
+~~~~~~
+
+``config`` is a primary firewall action used to apply the setting of the ``config``
+rule keyword when the rule matches, see :doc:`../rules/config`.
+The ``config`` action does not issue a verdict for the packet or the flow, so the
+other tables are still evaluated. It is not available as a secondary action.
+
 Multi action rules
 ~~~~~~~~~~~~~~~~~~
 

rules/rfb-events.rules

@@ -6,3 +6,4 @@ alert rfb any any -> any any (msg:"SURICATA RFB Malformed or unknown message"; a
 alert rfb any any -> any any (msg:"SURICATA RFB Unimplemented security type"; app-layer-event:rfb.unimplemented_security_type; classtype:protocol-command-decode; sid:2233001; rev:1;)
 alert rfb any any -> any any (msg:"SURICATA RFB Unknown security result"; app-layer-event:rfb.unknown_security_result; classtype:protocol-command-decode; sid:2233002; rev:1;)
 alert rfb any any -> any any (msg:"SURICATA RFB Unexpected State in Parser"; app-layer-event:rfb.confused_state; classtype:protocol-command-decode; sid:2233003; rev:1;)
+alert rfb any any -> any any (msg:"SURICATA RFB too long string"; app-layer-event:rfb.too_long_string; classtype:protocol-command-decode; sid:2233004; rev:1;)

rust/src/rfb/logger.rs

@@ -81,7 +81,10 @@ fn log_rfb(tx: &RFBTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> {
     js.close()?; // Close authentication.
 
     if let Some(ref reason) = tx.tc_failure_reason {
-        js.set_string("server_security_failure_reason", &reason.reason_string)?;
+        js.set_string(
+            "server_security_failure_reason",
+            &String::from_utf8_lossy(&reason.reason_string),
+        )?;
     }
 
     // Client/Server init

rust/src/rfb/parser.rs

@@ -93,7 +93,8 @@ pub struct SecurityResult {
 }
 
 pub struct FailureReason {
-    pub reason_string: String,
+    pub reason_string: Vec<u8>,
+    pub to_skip: u32,
 }
 
 pub struct VncAuth {
@@ -123,6 +124,7 @@ pub struct ServerInit {
     pub pixel_format: PixelFormat,
     pub name_length: u32,
     pub name: Vec<u8>,
+    pub to_skip: u32,
 }
 
 pub fn parse_protocol_version(i: &[u8]) -> IResult<&[u8], ProtocolVersion> {
@@ -177,13 +179,15 @@ pub fn parse_security_result(i: &[u8]) -> IResult<&[u8], SecurityResult> {
     Ok((i, SecurityResult { status }))
 }
 
-pub fn parse_failure_reason(i: &[u8]) -> IResult<&[u8], FailureReason> {
+pub fn parse_failure_reason(i: &[u8], max_len: u32) -> IResult<&[u8], FailureReason> {
     let (i, reason_length) = be_u32(i)?;
-    let (i, reason_string) = map_res(take(reason_length as usize), str::from_utf8).parse(i)?;
+    let to_skip = reason_length.saturating_sub(max_len);
+    let (i, reason_string) = take((reason_length - to_skip) as usize).parse(i)?;
     Ok((
         i,
         FailureReason {
-            reason_string: reason_string.to_string(),
+            reason_string: reason_string.to_vec(),
+            to_skip,
         },
     ))
 }
@@ -220,18 +224,21 @@ pub fn parse_pixel_format(i: &[u8]) -> IResult<&[u8], PixelFormat> {
     Ok((i, format))
 }
 
-pub fn parse_server_init(i: &[u8]) -> IResult<&[u8], ServerInit> {
+pub fn parse_server_init(i: &[u8], max_len: u32) -> IResult<&[u8], ServerInit> {
     let (i, width) = be_u16(i)?;
     let (i, height) = be_u16(i)?;
     let (i, pixel_format) = parse_pixel_format(i)?;
     let (i, name_length) = be_u32(i)?;
-    let (i, name) = take(name_length as usize)(i)?;
+    let to_skip = name_length.saturating_sub(max_len);
+
+    let (i, name) = take((name_length - to_skip) as usize)(i)?;
     let init = ServerInit {
         width,
         height,
         pixel_format,
         name_length,
         name: name.to_vec(),
+        to_skip,
     };
     Ok((i, init))
 }
@@ -273,7 +280,7 @@ mod tests {
             0x2e, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x64, 0x6f, 0x6d, 0x61, 0x69, 0x6e,
         ];
 
-        let result = parse_server_init(&buf);
+        let result = parse_server_init(&buf, 4096);
         match result {
             Ok((remainder, message)) => {
                 // Check the first message.

rust/src/rfb/rfb.rs

@@ -21,6 +21,7 @@
 use super::parser;
 use crate::applayer;
 use crate::applayer::*;
+use crate::conf::{conf_get, get_memval};
 use crate::core::{
     sc_app_layer_parser_trigger_raw_stream_inspection, ALPROTO_UNKNOWN, IPPROTO_TCP,
 };
@@ -38,13 +39,17 @@ use suricata_sys::sys::{
 };
 
 pub(super) static mut ALPROTO_RFB: AppProto = ALPROTO_UNKNOWN;
+// Maximum strings length in bytes.
+// If some string exceeds this length, it will be truncated.
+static mut MAX_STR_LEN: u32 = 4096;
 
 #[derive(FromPrimitive, Debug, AppLayerEvent)]
 pub enum RFBEvent {
     UnimplementedSecurityType,
     UnknownSecurityResult,
     MalformedMessage,
     ConfusedState,
+    TooLongString,
 }
 
 #[derive(AppLayerFrameType)]
@@ -116,6 +121,7 @@ pub struct RFBState {
     tx_id: u64,
     transactions: Vec<RFBTransaction>,
     state: parser::RFBGlobalState,
+    to_skip_tc: u32,
 }
 
 impl State<RFBTransaction> for RFBState {
@@ -141,6 +147,7 @@ impl RFBState {
             tx_id: 0,
             transactions: Vec::new(),
             state: parser::RFBGlobalState::TCServerProtocolVersion,
+            to_skip_tc: 0,
         }
     }
 
@@ -426,9 +433,16 @@ impl RFBState {
         if input.is_empty() {
             return AppLayerResult::ok();
         }
-
-        let mut current = input;
         let mut consumed = 0;
+        let mut current = input;
+        if self.to_skip_tc >= input.len() as u32 {
+            self.to_skip_tc -= input.len() as u32;
+            return AppLayerResult::ok();
+        } else if self.to_skip_tc > 0 {
+            consumed += self.to_skip_tc as usize;
+            current = &current[self.to_skip_tc as usize..];
+            self.to_skip_tc = 0;
+        }
         SCLogDebug!(
             "response_state {}, response_len {}",
             self.state,
@@ -709,9 +723,15 @@ impl RFBState {
                     }
                 }
                 parser::RFBGlobalState::TCFailureReason => {
-                    match parser::parse_failure_reason(current) {
-                        Ok((_rem, request)) => {
+                    match parser::parse_failure_reason(current, unsafe { MAX_STR_LEN }) {
+                        Ok((rem, request)) => {
+                            if request.to_skip >= rem.len() as u32 {
+                                self.to_skip_tc = request.to_skip - rem.len() as u32;
+                            }
                             if let Some(current_transaction) = self.get_current_tx() {
+                                if request.to_skip > 0 {
+                                    current_transaction.set_event(RFBEvent::TooLongString);
+                                }
                                 current_transaction.tc_failure_reason = Some(request);
                                 sc_app_layer_parser_trigger_raw_stream_inspection(
                                     flow,
@@ -740,23 +760,35 @@ impl RFBState {
                     }
                 }
                 parser::RFBGlobalState::TCServerInit => {
-                    match parser::parse_server_init(current) {
+                    match parser::parse_server_init(current, unsafe { MAX_STR_LEN }) {
                         Ok((rem, request)) => {
                             consumed += current.len() - rem.len();
                             let _pdu = Frame::new(
                                 flow,
                                 &stream_slice,
                                 current,
-                                consumed as i64,
+                                consumed as i64 + request.to_skip as i64,
                                 RFBFrameType::Pdu as u8,
                                 None,
                             );
 
-                            current = rem;
+                            if request.to_skip >= rem.len() as u32 {
+                                current = &rem[rem.len()..];
+                                consumed += rem.len();
+                                self.to_skip_tc = request.to_skip - rem.len() as u32;
+                            } else if request.to_skip > 0 {
+                                current = &rem[request.to_skip as usize..];
+                                consumed += request.to_skip as usize;
+                            } else {
+                                current = rem;
+                            }
 
                             self.state = parser::RFBGlobalState::Skip;
 
                             if let Some(current_transaction) = self.get_current_tx() {
+                                if request.to_skip > 0 {
+                                    current_transaction.set_event(RFBEvent::TooLongString);
+                                }
                                 current_transaction.tc_server_init = Some(request);
                                 sc_app_layer_parser_trigger_raw_stream_inspection(
                                     flow,
@@ -950,6 +982,17 @@ pub unsafe extern "C" fn SCRfbRegisterParser() {
         {
             SCLogDebug!("Failed to register protocol detection pattern for direction TOCLIENT");
         }
+        if let Some(val) = conf_get("app-layer.protocols.rfb.max-string-length") {
+            if let Ok(v) = get_memval(val) {
+                if v <= u32::MAX.into() {
+                    MAX_STR_LEN = v as u32;
+                } else {
+                    SCLogWarning!("rfb.max-string-length max is {}", u32::MAX);
+                }
+            } else {
+                SCLogWarning!("Invalid value for rfb.max-string-length: {}", val);
+            }
+        }
     } else {
         SCLogDebug!("Protocol detector and parser disabled for RFB.");
     }

src/detect-engine-analyzer.c

@@ -2146,46 +2146,70 @@ void EngineAnalysisRules(const DetectEngineCtx *de_ctx,
 
 #include "app-layer-parser.h"
 
-static void AddPolicy(const DetectEngineCtx *de_ctx, RuleAnalyzer *ctx, const AppProto a,
-        const uint8_t sub_state, const uint8_t state, const uint8_t direction)
+/**
+ * \brief Render a resolved firewall default policy as "<action>:<scope>".
+ *
+ * \retval true \p out holds the rendered policy
+ * \retval false the policy could not be rendered
+ */
+static bool FirewallPolicyToString(
+        const struct DetectFirewallPolicy *p, char *out, const size_t out_size)
 {
-    char policy_string[64] = "";
-    const struct DetectFirewallPolicies *fw_policies = de_ctx->fw_policies;
-    const struct DetectFirewallAppPolicy lookup = {
-        .alproto = a, .sub_state = sub_state, .progress = state, .direction = direction
-    };
-    const struct DetectFirewallAppPolicy *ap =
-            HashTableLookup(fw_policies->app_policies, (void *)&lookup, 0);
-    if (ap == NULL)
-        return;
-    const struct DetectFirewallPolicy *p = &ap->policy;
-
     const char *as = ActionScopeToString(p->action_scope);
     DEBUG_VALIDATE_BUG_ON(as == NULL);
     if (as == NULL)
-        return;
+        return false;
     if (p->action & ACTION_REJECT_ANY) {
         if (p->action & ACTION_REJECT_DST) {
-            snprintf(policy_string, sizeof(policy_string), "rejectdst:%s", as);
+            snprintf(out, out_size, "rejectdst:%s", as);
         } else if (p->action & ACTION_REJECT_BOTH) {
-            snprintf(policy_string, sizeof(policy_string), "rejectboth:%s", as);
+            snprintf(out, out_size, "rejectboth:%s", as);
         } else {
-            snprintf(policy_string, sizeof(policy_string), "rejectsrc:%s", as);
+            snprintf(out, out_size, "rejectsrc:%s", as);
         }
     } else if (p->action & ACTION_DROP) {
-        snprintf(policy_string, sizeof(policy_string), "drop:%s", as);
+        snprintf(out, out_size, "drop:%s", as);
     } else if (p->action & ACTION_ACCEPT) {
-        snprintf(policy_string, sizeof(policy_string), "accept:%s", as);
+        snprintf(out, out_size, "accept:%s", as);
     } else {
         DEBUG_VALIDATE_BUG_ON(1);
+        return false;
     }
     if (p->action & ACTION_PASS) {
-        if (p->action_scope == ACTION_SCOPE_FLOW) {
-            strlcat(policy_string, ",pass:flow", sizeof(policy_string));
+        if (p->action_scope == ACTION_SCOPE_FLOW || p->action_scope == ACTION_SCOPE_PACKET) {
+            if (strlcat(out, ",pass:", out_size) >= out_size ||
+                    strlcat(out, as, out_size) >= out_size) {
+                DEBUG_VALIDATE_BUG_ON(1);
+                return false;
+            }
         } else {
             DEBUG_VALIDATE_BUG_ON(1);
+            return false;
         }
     }
+    if (p->action & ACTION_ALERT) {
+        if (strlcat(out, ",alert", out_size) >= out_size) {
+            DEBUG_VALIDATE_BUG_ON(1);
+            return false;
+        }
+    }
+    return true;
+}
+
+static void AddPolicy(const DetectEngineCtx *de_ctx, RuleAnalyzer *ctx, const AppProto a,
+        const uint8_t sub_state, const uint8_t state, const uint8_t direction)
+{
+    char policy_string[64] = "";
+    const struct DetectFirewallPolicies *fw_policies = de_ctx->fw_policies;
+    const struct DetectFirewallAppPolicy lookup = {
+        .alproto = a, .sub_state = sub_state, .progress = state, .direction = direction
+    };
+    const struct DetectFirewallAppPolicy *ap =
+            HashTableLookup(fw_policies->app_policies, (void *)&lookup, 0);
+    if (ap == NULL)
+        return;
+    if (!FirewallPolicyToString(&ap->policy, policy_string, sizeof(policy_string)))
+        return;
     SCJbSetString(ctx->js, "policy", policy_string);
 }
 
@@ -2258,7 +2282,11 @@ int FirewallAnalyzer(const DetectEngineCtx *de_ctx)
 
     SCJbOpenObject(ctx.js, "tables");
     SCJbOpenObject(ctx.js, "packet:filter");
-    SCJbSetString(ctx.js, "policy", "drop:packet");
+    char pkt_policy[64] = "";
+    if (FirewallPolicyToString(&de_ctx->fw_policies->pkt[DETECT_FIREWALL_POLICY_PACKET_FILTER],
+                pkt_policy, sizeof(pkt_policy))) {
+        SCJbSetString(ctx.js, "policy", pkt_policy);
+    }
     SCJbOpenArray(ctx.js, "rules");
     uint32_t accept_rules = 0;
     uint32_t last_sid = 0;