fixes a use-of-uninitialized-value in light_pcapng.c #1669
Conversation
|
Hii @seladb Could You please Review this PR |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1669 +/- ##
==========================================
- Coverage 83.15% 83.15% -0.01%
==========================================
Files 277 277
Lines 48193 48204 +11
Branches 9949 9975 +26
==========================================
+ Hits 40074 40083 +9
+ Misses 7226 7223 -3
- Partials 893 898 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| } | ||
| break; | ||
| void parse_by_block_type(struct _light_pcapng *current, const uint32_t *local_data, const uint32_t *block_start) { | ||
| switch (current->block_type) { |
There was a problem hiding this comment.
It's better to follow the original format.
There was a problem hiding this comment.
Thank you for reviewing Could you clarify if the concern is solely about formatting or if there are issues with the fix itself? If it’s just formatting, I’ll align it with the original style while keeping the vulnerability fix intact.
There was a problem hiding this comment.
Only about the formatting. We try to minimize the change in 3rdParty because we may merge the latest changes from upstream in the future. However, vulnerability fixes are welcome. Please also leave a comment like // PCPP patch in your patch so we can easily identify the changes in the future.
There was a problem hiding this comment.
Yes Done you can Review and Ready to Merge
| { // PCPP patch | ||
| DPRINT_HERE(LIGHT_SECTION_HEADER_BLOCK); | ||
| struct _light_section_header *shb = calloc(1, sizeof(struct _light_section_header)); | ||
| struct _light_option *opt = calloc(1, sizeof(struct _light_option)); |
There was a problem hiding this comment.
Doesn't that esentially leak the memory as opt is overwritten on L109?
There was a problem hiding this comment.
Yes you are correct for this here can be used free (opt) Memory Before Reassignment
I had created Commit below
There was a problem hiding this comment.
Why is it allocated here actually, if it will be freed in L110? I don't see any usages of the pointer in between?
|
|
||
| case LIGHT_ENHANCED_PACKET_BLOCK: | ||
| { | ||
| DPRINT_HERE(LIGHT_ENHANCED_PACKET_BLOCK); | ||
| struct _light_enhanced_packet_block *epb = NULL; | ||
| struct _light_option *opt = NULL; | ||
| struct _light_option *opt = calloc(1, sizeof(struct _light_option)); |
| @@ -182,7 +181,7 @@ void parse_by_block_type(struct _light_pcapng *current, const uint32_t *local_da | |||
| { | |||
| DPRINT_HERE(LIGHT_CUSTOM_DATA_BLOCK); | |||
| struct _light_custom_nonstandard_block *cnb = NULL; | |||
| struct _light_option *opt = NULL; | |||
| struct _light_option *opt = calloc(1, sizeof(struct _light_option)); | |||
There was a problem hiding this comment.
Yes done some Improvement to memory free(opt) now there wouldn't be any leakage i think
|
@Shivam7-1 Could you only modify the necessary parts? Try to minimiaze the lines of changes ... |
| current->options = opt; | ||
| } | ||
| break; | ||
| void parse_by_block_type(struct _light_pcapng *current, const uint32_t *local_data, const uint32_t *block_start) { |
There was a problem hiding this comment.
Don't change the format. Same for all other lines.
Hii where I could see CI issue or find out |
This PR fixes a use-of-uninitialized-value vulnerability in light_pcapng.c. The changes ensure all allocated memory is properly initialized before use, mitigating potential crashes and undefined behavior.
Issue: https://issues.oss-fuzz.com/issues/42536106
Reproducer Testcase: https://oss-fuzz.com/download?testcase_id=6639796737867776