Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes a use-of-uninitialized-value in light_pcapng.c #1669

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
+633 −579
Open

fixes a use-of-uninitialized-value in light_pcapng.c #1669

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
60323c3
Update light_pcapng.c
Shivam7-1 Dec 23, 2024
cd2cb51
Fixes Formatting issues
Shivam7-1 Dec 25, 2024
88659d7
Freed opt Memory Before Reassignment
Shivam7-1 Dec 25, 2024
9f355b1
Fixes memory leakages
Shivam7-1 Dec 25, 2024
a438dcc
Merge branch 'dev' into patch-1
Shivam7-1 Dec 27, 2024
86ed6af
formating has been done using prettier
Shivam7-1 Dec 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 58 additions & 64 deletions 3rdParty/LightPcapNg/LightPcapNg/src/light_pcapng.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,56 +85,55 @@ static struct _light_option *__parse_options(uint32_t **memory, const int32_t ma
/// <param name="current">Block pointer with type and length filled out</param>
/// <param name="local_data">Pointer to data which constitutes block body</param>
/// <param name="block_start">Pointer to the start of the block data</param>
void parse_by_block_type(struct _light_pcapng *current, const uint32_t *local_data, const uint32_t *block_start)
{
switch (current->block_type)
{
case LIGHT_SECTION_HEADER_BLOCK:
{
DPRINT_HERE(LIGHT_SECTION_HEADER_BLOCK);
struct _light_section_header *shb = calloc(1, sizeof(struct _light_section_header));
struct _light_option *opt = NULL;
uint32_t version;
int32_t local_offset;

shb->byteorder_magic = *local_data++;
// TODO check byte order.
version = *local_data++;
shb->major_version = version & 0xFFFF;
shb->minor_version = (version >> 16) & 0xFFFF;
shb->section_length = *((uint64_t*)local_data);
local_data += 2;

current->block_body = (uint32_t*)shb;
local_offset = (size_t)local_data - (size_t)block_start;
opt = __parse_options((uint32_t **)&local_data, current->block_total_length - local_offset - sizeof(current->block_total_length));
current->options = opt;
}
break;

case LIGHT_INTERFACE_BLOCK:
{
DPRINT_HERE(LIGHT_INTERFACE_BLOCK);
struct _light_interface_description_block *idb = calloc(1, sizeof(struct _light_interface_description_block));
struct _light_option *opt = NULL;
uint32_t link_reserved = *local_data++;
int32_t local_offset;

idb->link_type = link_reserved & 0xFFFF;
idb->reserved = (link_reserved >> 16) & 0xFFFF;
idb->snapshot_length = *local_data++;
current->block_body = (uint32_t*)idb;
local_offset = (size_t)local_data - (size_t)block_start;
opt = __parse_options((uint32_t **)&local_data, current->block_total_length - local_offset - sizeof(current->block_total_length));
current->options = opt;
}
break;
void parse_by_block_type(struct _light_pcapng *current, const uint32_t *local_data, const uint32_t *block_start) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't change the format. Same for all other lines.

switch (current->block_type)
{
case LIGHT_SECTION_HEADER_BLOCK:
{ // 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));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't that esentially leak the memory as opt is overwritten on L109?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you are correct for this here can be used free (opt) Memory Before Reassignment
I had created Commit below

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it allocated here actually, if it will be freed in L110? I don't see any usages of the pointer in between?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had updated Recent commit

uint32_t version = 0;
int32_t local_offset = 0;

shb->byteorder_magic = *local_data++;
// TODO check byte order.
version = *local_data++;
shb->major_version = version & 0xFFFF;
shb->minor_version = (version >> 16) & 0xFFFF;
shb->section_length = *((uint64_t*)local_data);
local_data += 2;

current->block_body = (uint32_t*)shb;
local_offset = (size_t)local_data - (size_t)block_start;
opt = __parse_options((uint32_t **)&local_data, current->block_total_length - local_offset - sizeof(current->block_total_length));
current->options = opt;
}
break;

case LIGHT_INTERFACE_BLOCK:
{ // PCPP patch
DPRINT_HERE(LIGHT_INTERFACE_BLOCK);
struct _light_interface_description_block *idb = calloc(1, sizeof(struct _light_interface_description_block));
struct _light_option *opt = calloc(1, sizeof(struct _light_option));
Shivam7-1 marked this conversation as resolved.
Show resolved Hide resolved
uint32_t link_reserved = *local_data++;
int32_t local_offset = 0;

idb->link_type = link_reserved & 0xFFFF;
idb->reserved = (link_reserved >> 16) & 0xFFFF;
idb->snapshot_length = *local_data++;
current->block_body = (uint32_t*)idb;
local_offset = (size_t)local_data - (size_t)block_start;
opt = __parse_options((uint32_t **)&local_data, current->block_total_length - local_offset - sizeof(current->block_total_length));
current->options = opt;
}
break;

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));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto: memory leak

uint32_t interface_id = *local_data++;
uint32_t timestamp_high = *local_data++;
uint32_t timestamp_low = *local_data++;
Expand Down Expand Up @@ -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));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto: memory leak

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes done some Improvement to memory free(opt) now there wouldn't be any leakage i think

uint32_t len = *local_data++;
uint32_t reserved0 = *local_data++;
uint32_t reserved1 = *local_data++;
Expand All @@ -203,24 +202,19 @@ void parse_by_block_type(struct _light_pcapng *current, const uint32_t *local_da
current->options = opt;
}
break;

default: // Could not find registered block type. Copying data as RAW.
{
DPRINT_HERE(default);
uint32_t raw_size = current->block_total_length - 2 * sizeof(current->block_total_length) - sizeof(current->block_type);
if (raw_size > 0)
{
current->block_body = calloc(raw_size, 1);
memcpy(current->block_body, local_data, raw_size);
local_data += raw_size / (sizeof(*local_data));
}
else
{
current->block_body = NULL;
}
}
break;
}
default: {
DPRINT_HERE(default);
uint32_t raw_size = current->block_total_length - 2 * sizeof(current->block_total_length) - sizeof(current->block_type);
if (raw_size > 0) {
current->block_body = calloc(raw_size, 1);
memcpy(current->block_body, local_data, raw_size);
local_data += raw_size / (sizeof(*local_data));
} else {
current->block_body = NULL;
}
}
break;
}
}

// Parse memory and allocate _light_pcapng array.
Expand Down
Loading