decode: prometheus: fix to avoid freeing non-malloced data #187
Conversation
The fuzzer in fluent/fluent-bit#7745 found a bug that shows the prometheus decoder is sometimes freeing data that is not malloced. This is an attempt to fix that. Signed-off-by: David Korczynski <[email protected]>
| @@ -166,20 +160,21 @@ static int split_metric_name(struct cmt_decode_prometheus_context *context, | |||
| } | |||
| *subsystem = strchr(*ns, '_'); | |||
| if (!(*subsystem)) { | |||
| *name = *ns; | |||
| *ns = ""; | |||
| *name = strdup(*ns); | |||
There was a problem hiding this comment.
Please check this result.
| *ns = ""; | ||
| *name = strdup(*ns); | ||
| free(*ns); | ||
| *ns = strdup(""); |
There was a problem hiding this comment.
Please check this result.
| } | ||
| else { | ||
| **subsystem = 0; /* split */ | ||
| (*subsystem)++; | ||
| *name = strchr(*subsystem, '_'); | ||
| if (!(*name)) { | ||
| *name = *subsystem; | ||
| *name = strdup(*subsystem); |
There was a problem hiding this comment.
Please check this result.
| **name = 0; | ||
| (*name)++; | ||
| **name = '\0'; | ||
| *name = strdup((*name)++); |
There was a problem hiding this comment.
Please check this result. and if possible dereference name in this way (*name)[1].
|
ping @DavidKorczynski on changes requested |
|
I think the issue highlighted by this PR may need a more substantial fix. There is mixing of dynamically allocated memory and non-dynamically allocated memory, and in order to achieve consistency there is some rewriting needed (not just fixing). I think the owner/maintainer of the code should address the issues -- @tarruda can you assist here? |
|
@tarruda it seems this needs a more elaborated solution. would you please take a look at it ? (cc: @niedbalski for visibility) |
The fuzzer in fluent/fluent-bit#7745 found a bug that shows the prometheus decoder is sometimes freeing data that is not malloced. This is an attempt to fix that.
Please do verify this -- the fuzzer no longer finds the issue here and the bug is obvious, however, for coming up with a fix I found the code a bit tricky to validate as I'm not deeply familiar with this decoder.