Skip to content
This repository was archived by the owner on Jan 2, 2021. It is now read-only.

stringop-truncation and address-of-packed-member #309

Closed
NaruFGT wants to merge 1 commit into from
Closed

stringop-truncation and address-of-packed-member #309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion Libraries/libbom/Sources/bom.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ bom_alloc_empty(struct bom_context_memory memory)
context->memory.resize(&context->memory, header_size + index_size + freelist_size + variables_size);

struct bom_header *header = (struct bom_header *)context->memory.data;
strncpy(header->magic, "BOMStore", 8);
memcpy(header->magic, "BOMStore", 8);
header->version = htonl(1);
header->block_count = htonl(0);
header->index_offset = htonl(header_size);
Expand Down
2 changes: 1 addition & 1 deletion Libraries/libbom/Sources/bom_tree.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ bom_tree_alloc_empty(struct bom_context *context, const char *variable_name)
uint32_t entry_index = bom_index_add(tree_context->context, entry, sizeof(*entry));
free(entry);

strncpy(tree->magic, "tree", 4);
memcpy(tree->magic, "tree", 4);
tree->version = htonl(1);
tree->child = htonl(entry_index);
tree->node_size = htonl(4096); // todo
Expand Down
12 changes: 10 additions & 2 deletions Libraries/libcar/Sources/Reader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ renditionIterate(std::function<void(Rendition const &)> const &iterator) const
KeyValuePair kv = (KeyValuePair)it.second;
car_rendition_key *rendition_key = (car_rendition_key *)kv.key;
struct car_rendition_value *rendition_value = (struct car_rendition_value *)kv.value;
AttributeList attributes = AttributeList::Load(keyfmt->num_identifiers, keyfmt->identifier_list, rendition_key);
uint32_t *unpacked_identifiers = new uint32_t[sizeof(uint32_t)*keyfmt->num_identifiers];
for(size_t i = 0; i < keyfmt->num_identifiers; ++i) {
unpacked_identifiers[i] = keyfmt->identifier_list[i];
}
AttributeList attributes = AttributeList::Load(keyfmt->num_identifiers, unpacked_identifiers, rendition_key);
Comment on lines +85 to +89
Copy link
Contributor

@sas sas Dec 30, 2019

Choose a reason for hiding this comment

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

I think there's an alignment problem here.

Rendition rendition = Rendition::Load(attributes, rendition_value);
iterator(rendition);
}
Expand Down Expand Up @@ -264,7 +268,11 @@ lookupRenditions(Facet const &facet) const
KeyValuePair value = (KeyValuePair)it->second;
car_rendition_key *rendition_key = (car_rendition_key *)value.key;
struct car_rendition_value *rendition_value = (struct car_rendition_value *)value.value;
AttributeList attributes = AttributeList::Load(keyfmt->num_identifiers, keyfmt->identifier_list, rendition_key);
uint32_t *unpacked_identifiers = new uint32_t[sizeof(uint32_t)*keyfmt->num_identifiers];
Copy link
Contributor

@sas sas Dec 30, 2019

Choose a reason for hiding this comment

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

You're never freeing the memory block you allocated here. Also, the size you allocate seems wrong to me; you're gonna end up allocating 4 entries in your array (sizeof(uint32_t)).

for(size_t i = 0; i < keyfmt->num_identifiers; ++i) {
unpacked_identifiers[i] = keyfmt->identifier_list[i];
}
AttributeList attributes = AttributeList::Load(keyfmt->num_identifiers, unpacked_identifiers, rendition_key);
Comment on lines +271 to +275
Copy link
Contributor

@sas sas Dec 30, 2019

Choose a reason for hiding this comment

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

Formatting.

Rendition rendition = Rendition::Load(attributes, rendition_value);
result.push_back(rendition);
}
Expand Down
4 changes: 2 additions & 2 deletions Libraries/libcar/Sources/Rendition.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ write() const
// Create header
struct car_rendition_value header;
memset(static_cast<void *>(&header), 0, sizeof(struct car_rendition_value));
strncpy(header.magic, "ISTC", 4);
memcpy(header.magic, "ISTC", 4);
header.version = 1;
// header.flags.is_header_flagged_fpo = 0;
// header.flags.is_excluded_from_contrast_filter = 0;
Expand Down Expand Up @@ -675,7 +675,7 @@ write() const
/* JPEG, RAW and non-standard formats adds one more header at the end */
if (Rendition::Data::FormatSavedAsRawData(renditionData->format())) {
struct car_rendition_data_header_raw raw_header;
strncpy(raw_header.magic, "DWAR", 4);
memcpy(raw_header.magic, "DWAR", 4);
raw_header.length = compressed_data_length;
memcpy(output_bytes, &raw_header, sizeof(struct car_rendition_data_header_raw));
output_bytes += sizeof(struct car_rendition_data_header_raw);
Expand Down
10 changes: 7 additions & 3 deletions Libraries/libcar/Sources/Writer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ write() const
return;
}

strncpy(header->magic, "RATC", 4);
memcpy(header->magic, "RATC", 4);
header->ui_version = 0x131; // TODO
header->storage_version = 0xC; // TODO
header->storage_timestamp = static_cast<uint32_t>(time(NULL)); // TODO
Expand Down Expand Up @@ -127,7 +127,7 @@ write() const
std::vector<enum car_attribute_identifier> format = DetermineKeyFormat(_facets, _renditions);
keyfmt_size = sizeof(struct car_key_format) + (format.size() * sizeof(uint32_t));
keyfmt = (struct car_key_format *)malloc(keyfmt_size);
strncpy(keyfmt->magic, "tmfk", 4);
memcpy(keyfmt->magic, "tmfk", 4);
keyfmt->reserved = 0;
keyfmt->num_identifiers = format.size();
for (size_t i = 0; i < format.size(); ++i) {
Expand Down Expand Up @@ -162,7 +162,11 @@ write() const
bom_tree_reserve(renditions_tree_context, rendition_count);
if (renditions_tree_context != NULL) {
for (auto const &item : _renditions) {
auto attributes_value = item.second.attributes().write(keyfmt->num_identifiers, keyfmt->identifier_list);
uint32_t *unpacked_identifiers = new uint32_t[sizeof(uint32_t)*keyfmt->num_identifiers];
Copy link
Contributor

@sas sas Dec 30, 2019

Choose a reason for hiding this comment

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

Memory leak + incorrect size specifier.

for(size_t i = 0; i < keyfmt->num_identifiers; ++i) {
unpacked_identifiers[i] = keyfmt->identifier_list[i];
}
auto attributes_value = item.second.attributes().write(keyfmt->num_identifiers, unpacked_identifiers);
Comment on lines +165 to +169
Copy link
Contributor

@sas sas Dec 30, 2019

Choose a reason for hiding this comment

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

Formatting.

auto rendition_value = item.second.write();
bom_tree_add(
renditions_tree_context,
Expand Down