Project

General

Profile

Actions

Security #6286

closed
BJ PA

lua/tls: null dereference in GetCertInfo

Security #6286: lua/tls: null dereference in GetCertInfo

Added by Ben Jackson almost 3 years ago. Updated about 11 hours ago.

Status:
Closed
Priority:
Normal
Target version:
Affected Versions:
Label:
Hardening, Needs Suricata-Verify test
Git IDs:

20fa5d773fd8ccebaa58294f59fb6cd50f55c4d5

Severity:
HIGH
Disclosure Date:

Description

I'm currently detecting the use of self-signed certificates using a rule similar to https://www.stamus-networks.com/blog/2015/07/24/finding-self-signed-tls-certificates-suricata-and-luajit-scripting

In certain scenarios -- I'm still not sure which -- it appears that certain members of SSLStateConnp are not populated, and instead contain null pointers. More specifically, in my case, `connp->cert0_issuerdn`. When this is the case and the rule I mentioned is processed, it results in a strlen(NULL) call and thus a segfault as a result of dereferencing a null pointer.

In other parts of the code (e.g. https://github.com/OISF/suricata/blob/becb8cefccc10c352184a9e8e930fd5fa9652651/src/detect-tls-cert-issuer.c#L138 ) these struct members are checked for NULL before usage. This should probably be the case here too.

I'm running 6.0.9, but the same code has appeared in a much wider set of releases -- seemingly since this code was first added in https://github.com/OISF/suricata/commit/371648a8c61e93b42f74263bcedb9d1b8b1af354#diff-5e8f4c32849c58cd5e3fe9786a3b2e818c8a3567211029b8dcbae48bb2fcd0b7


Related issues 1 (0 open1 closed)

Related to Suricata - Bug #7887: detect/tls: zero characters in keywords such as alt name are mishandledClosedShivani BhardwajActions

JI Updated by Jason Ish almost 3 years ago Actions #1

  • Private changed from No to Yes

JF Updated by Juliana Fajardini Reichow over 2 years ago Actions #2

  • Label Hardening added

PA Updated by Philippe Antoine over 2 years ago Actions #3

Would you have some suricata-verify test to reproduce ?
This requires Lua scripts, right ?

PA Updated by Philippe Antoine 11 months ago Actions #4

  • Label Needs Suricata-Verify test added

PA Updated by Philippe Antoine 11 months ago Actions #5

  • Status changed from New to Feedback

PA Updated by Philippe Antoine 11 months ago Actions #6

  • Status changed from Feedback to New
  • Affected Versions 8.0.0 added
  • Affected Versions deleted (TBD)

Code looks bad indeed

PA Updated by Philippe Antoine about 2 months ago Actions #7

  • Tracker changed from Bug to Security
  • Effort deleted (low)

Promoting to security issue to get a closer look

PA Updated by Philippe Antoine about 1 month ago Actions #8

Maybe fixed by f025e0719193d646d20c8f2d384da7f61be638f2 for 8 and 9

PA Updated by Philippe Antoine about 1 month ago Actions #9

  • Related to Bug #7887: detect/tls: zero characters in keywords such as alt name are mishandled added

PA Updated by Philippe Antoine about 1 month ago Actions #10

Reproducing on main7 with
- a pcap where the issuer has a zero byte
- rule `alert tls any any > any any (msg:"lua test"; lua:tls.lua; sid:3;)`
--set security.lua.allow-rules=true

tls.lua being

function init(args)
    local needs = {}
    needs["tls"] = tostring(true)
    return needs
end

function match(args)
    version, subject, issuer, fingerprint = TlsGetCertInfo();
    if subject == issuer then
        return 1
    end
    return 0
end

PA Updated by Philippe Antoine about 1 month ago Actions #11

  • Status changed from New to In Review
  • Assignee changed from OISF Dev to Philippe Antoine
  • Target version changed from TBD to 7.0.16

Gitlab MR

PA Updated by Philippe Antoine about 1 month ago Actions #12

  • Subject changed from Segmentation fault in GetCertInfo -- Missing null pointer checks to lua/tls: null dereference in GetCertInfo

JI Updated by Jason Ish 23 days ago Actions #13

  • GHSA set to GHSA-vfc5-9844-rmhv

JI Updated by Jason Ish 23 days ago Actions #14

  • Severity set to HIGH

PA Updated by Philippe Antoine 21 days ago Actions #15

  • CVE set to 2026-45747

PA Updated by Philippe Antoine 16 days ago Actions #16

  • Status changed from In Review to Resolved

Staged

VJ Updated by Victor Julien 15 days ago Actions #17

  • Status changed from Resolved to Closed

JI Updated by Jason Ish 15 days ago Actions #18

  • Git IDs updated (diff)

JI Updated by Jason Ish about 11 hours ago Actions #19

  • Private changed from Yes to No
Actions

Also available in: PDF Atom