Project

General

Profile

Actions

Bug #8651

open
PA PA

conf: null deref when using YAML null values

Bug #8651: conf: null deref when using YAML null values

Added by Philippe Antoine about 3 hours ago. Updated about 2 hours ago.

Status:
In Review
Priority:
Normal
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

Original report

I am reporting an API contract violation in SCConfGet() that causes Suricata to crash with SIGSEGV on startup when any of 30+ configuration keys are set to a YAML null value (~, null, Null, NULL, or
  empty unquoted).

  Affected version: 8.0.5 (latest)
  File: src/conf.c, line 354–363

  ---
  Root cause

  SCConfGet() returns 1 (found) when the configuration node exists but node->val is NULL — which is the case for any YAML null value. Callers assume retval == 1 implies val != NULL and dereference
  directly:

  // src/conf.c:354-363
  int SCConfGet(const char *name, const char **vptr)
  {
      SCConfNode *node = SCConfGetNode(name);
      if (node == NULL) {
          return 0;
      }
      else {
          *vptr = node->val;   // ← val may be NULL
          return 1;
      }
  }

  The YAML loader (conf-yaml-loader.c:227-232) explicitly converts unquoted ~, null, Null, NULL, and empty values to C NULL, following YAML 1.1. SCConfGetChildValue() (conf.c:368-376) already checks
  node->val == NULL and returns 0. SCConfGet() does not — the inconsistency is the root cause.

  ---
  Evidence the issue is known

  1. src/flow.c:594-598 — FlowInitConfig() explicitly checks if (conf_val == NULL) { FatalError(...) } after SCConfGet(), a one-off patch that was never propagated to the other 30+ callers.
  2. src/conf-yaml-loader.c:993 — The ConfYamlNull unit test asserts FAIL_IF_NOT(SCConfGet("unquoted-null", &val)) + FAIL_IF_NOT_NULL(val) — the test suite explicitly documents that SCConfGet returns
  1 with val=NULL.

  ---
  Crash paths verified on 8.0.5

  Path 1 — ja3-fingerprints: ~ → strcmp(NULL, "auto") → SIGSEGV

  GDB backtrace:
  Program received signal SIGSEGV, Segmentation fault.
  __strcmp_avx2 () at strcmp-avx2.S:115
  #0  __strcmp_avx2 ()
  #1  CheckJA3Enabled () at app-layer-ssl.c:3142
  #2  RegisterSSLParsers () at app-layer-ssl.c:3282
  #3  AppLayerParserRegisterProtocolParsers () at app-layer-parser.c:1787
  #4  AppLayerSetup () at app-layer.c:1088
  #5  PostConfLoadedSetup () at suricata.c:2836
  #6  SuricataInit () at suricata.c:3078
  #7  main () at main.c:57

  Valgrind report:
  ==PID== Invalid read of size 1
  ==PID==    at 0x484FBD4: strcmp (vgpreload_memcheck)
  ==PID==    by 0x7E4BE2: CheckJA3Enabled (app-layer-ssl.c:3142)
  ==PID==    by 0x7E4BE2: RegisterSSLParsers (app-layer-ssl.c:3282)
  ==PID==    by 0x7D906B: AppLayerParserRegisterProtocolParsers (app-layer-parser.c:1787)
  ==PID==    by 0x7E849A: AppLayerSetup (app-layer.c:1088)
  ==PID==    by 0x79E49A: PostConfLoadedSetup (suricata.c:2836)
  ==PID==    by 0x79F6B9: SuricataInit (suricata.c:3078)
  ==PID==    by 0x79A522: main (main.c:57)
  ==PID==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

  Path 2 — defrag.prealloc: ~ → SCConfValIsTrue(NULL) → strcasecmp(NULL,...) → SIGSEGV

  GDB backtrace:
  Program received signal SIGSEGV, Segmentation fault.
  __strcasecmp_l_avx ()
  #0  __strcasecmp_l_avx ()
  #1  SCConfValIsTrue (val=0x0) at conf.c:558
  #2  DefragInitConfig () at defrag-hash.c:256
  #3  DefragInit () at defrag.c:1136
  #4  PreRunInit () at suricata.c:2319
  #5  PostConfLoadedSetup () at suricata.c:2941
  #6  SuricataInit () at suricata.c:3078
  #7  main () at main.c:57

  Note: val=0x0 is explicit in the frame — SCConfValIsTrue receives NULL directly.

  Path 3 — defrag.hash-size: ~ → strlen(NULL) → SIGSEGV

  GDB backtrace:
  Program received signal SIGSEGV, Segmentation fault.
  __strlen_avx2 () 
  #0  __strlen_avx2 ()
  #1  DefragInitConfig () at defrag-hash.c:204
  #2  DefragInit () at defrag.c:1136
  #3  PreRunInit () at suricata.c:2319
  #4  PostConfLoadedSetup () at suricata.c:2941
  #5  SuricataInit () at suricata.c:3078
  #6  main () at main.c:57

  Path 4 — stream.inline: ~ → strcmp(NULL, "auto") → SIGSEGV

  GDB backtrace:
  Program received signal SIGSEGV, Segmentation fault.
  __strcmp_avx2 () 
  #0  __strcmp_avx2 ()
  #1  StreamTcpInitConfig () at stream-tcp.c:591
  #2  PreRunInit () at suricata.c:2322
  #3  PostConfLoadedSetup () at suricata.c:2941
  #4  SuricataInit () at suricata.c:3078
  #5  main () at main.c:57

  ---
  Proposed fix

  Add one NULL check to SCConfGet(), consistent with SCConfGetChildValue():

  // src/conf.c:354-363 — proposed fix
  int SCConfGet(const char *name, const char **vptr)
  {
      SCConfNode *node = SCConfGetNode(name);
      if (node == NULL) {
          SCLogDebug("failed to lookup configuration parameter '%s'", name);
          return 0;
      }
      if (node->val == NULL) {        // ← add this line
          return 0;
      }
      *vptr = node->val;
      return 1;
  }

  This fixes all 30+ callers at once. The existing flow.c NULL check becomes unreachable (harmless). SCConfGetBool() no longer reaches SCConfValIsTrue(NULL).

Subtasks 1 (1 open0 closed)

Bug #8652: conf: null deref when using YAML null values (8.0.x backport)AssignedPhilippe AntoineActions

OT Updated by OISF Ticketbot about 3 hours ago Actions #1

  • Subtask #8652 added

OT Updated by OISF Ticketbot about 3 hours ago Actions #2

  • Label deleted (Needs backport to 8.0)

PA Updated by Philippe Antoine about 2 hours ago Actions #3

  • Status changed from Assigned to In Review
Actions

Also available in: PDF Atom