Project

General

Profile

Actions

Security #7300

open

log: too big record lead to invalid json

Added by Philippe Antoine about 2 months ago. Updated 17 days ago.

Status:
Assigned
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Label:
CVE:
Git IDs:
Severity:
MODERATE
Disclosure Date:

Subtasks 1 (1 open0 closed)

Security #7301: log: too big record lead to invalid json (7.0.x backport)AssignedOISF DevActions
Actions #1

Updated by OISF Ticketbot about 2 months ago

  • Subtask #7301 added
Actions #2

Updated by OISF Ticketbot about 2 months ago

  • Label deleted (Needs backport to 7.0)
Actions #3

Updated by Philippe Antoine about 2 months ago

Summing up the relevant part of the parent issue :

A first fix is to simply check the return value of MemBufferExpand in OutputJsonBuilderBuffer

how do we handle a log record that is over the buffer size ? (10 MB)

Also, should we try to avoid these ? I think so...

The JSON buffer is already complete, so we'd have to lightly parse it to know how to do that. Funny that its already in memory, but its the abstraction to underlying writers where we hit this limit.

So, looks like we should enforce this limit sooner, right ?

IMO, if we decode it we should probably log it, enforce limits in the decoding, but if it makes it to the logger, and then deem it too big maybe some special engine event.. Just thinking out loud. Or better yet, if it's logging a JsonBuilder, using the built buffer directly instead of copying?

For HTTP2, the solution was to log only log once a repeated header.
I know that DNS model is not the same... But still gives ideas

Actions #4

Updated by Jason Ish about 2 months ago ยท Edited

This was just a quick POC to use the JsonBuilder buffer directly instead of memcpy, which does fix this. But still needs more investigation: https://github.com/OISF/suricata/pull/11864

Actions #5

Updated by Philippe Antoine about 2 months ago

This was just a quick POC to use the JsonBuilder buffer directly instead of memcpy

Looks cool...

But do we want to allow 10Mbytes records ?

Actions #6

Updated by Victor Julien 17 days ago

  • Status changed from New to Assigned
  • Assignee changed from OISF Dev to Jason Ish

For 8, we are thinking of:
1. get rid double buffering
2. add limit to jsonbuilder in "root" object open call
3. jsonbuilder skips adding fields that would make it exceed the limit
4. jsonbuilder root object would hold a bit of info: like first field name that tripped the limit
5. special root object close call will return warning/error for logging, and close the partial object correctly

Actions #7

Updated by Victor Julien 17 days ago

Additional thoughts: current 10MiB is very large and should probably be reduced. Also should perhaps be configurable. Maybe as a jb_set_limit call or similar.

Actions

Also available in: Atom PDF