Security #7300
openlog: too big record lead to invalid json
Updated by OISF Ticketbot about 2 months ago
- Label deleted (
Needs backport to 7.0)
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
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
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 ?
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
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.