diffs to fix buffer overflows #32
Description
Subject: [PATCH 1/2] fix buffer length calculation
---
Src/os/posix.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Src/os/posix.c b/Src/os/posix.c
index 009c509..3c5e694 100644
--- a/Src/os/posix.c
+++ b/Src/os/posix.c
@@ -77,7 +77,8 @@ int os_GetFolderFiles(char *folder_path, char *hierarchy)
if (entry == NULL) break;
// +2 for \0 and a possible slash
- char *heap_path = calloc(1, strlen(folder_path) + 2);
+ size_t heap_path_length = strlen(folder_path) + strlen(entry->d_name) + 2;
+ char *heap_path = calloc(1, heap_path_length);
strcpy(heap_path, folder_path);
// If there's no trailing dir slash, we append it
--
2.24.3 (Apple Git-128)
--
Subject: [PATCH 2/2] fix OOB accesses in AppleSingle parsing
---
Src/File_AppleSingle.c | 83 ++++++++++++++++++++++--------------------
Src/File_AppleSingle.h | 14 +++----
Src/Prodos_Add.c | 4 +-
3 files changed, 52 insertions(+), 49 deletions(-)
diff --git a/Src/File_AppleSingle.c b/Src/File_AppleSingle.c
index 3973e6a..87263ca 100644
--- a/Src/File_AppleSingle.c
+++ b/Src/File_AppleSingle.c
@@ -20,8 +20,10 @@ static uint16_t as_field16(uint16_t num)
* @param buf
* @return
*/
-bool ASIsAppleSingle(unsigned char *buf)
+bool ASIsAppleSingle(unsigned char *buf, size_t buflen)
{
+ if (buflen < sizeof(AS_MAGIC)) return false;
+
int buf_magic;
memcpy(&buf_magic, buf, sizeof(AS_MAGIC));
buf_magic = as_field32(buf_magic);
@@ -35,11 +37,14 @@ bool ASIsAppleSingle(unsigned char *buf)
* @param buf The buffer
* @return
*/
-struct as_file_header *ASParseHeader(unsigned char *buf)
+struct as_file_header *ASParseHeader(unsigned char *buf, size_t buflen)
{
- struct as_file_header *header = malloc(sizeof(as_file_header));
- struct as_file_header *buf_header = (as_file_header *) buf;
+ if (buflen < sizeof(as_file_header)) return NULL;
+ struct as_file_header *header = calloc(1, sizeof(as_file_header));
+ if (!header) return NULL;
+
+ struct as_file_header *buf_header = (as_file_header *) buf;
header->magic = as_field32(buf_header->magic);
header->version = as_field32(buf_header->version);
header->num_entries = as_field16(buf_header->num_entries);
@@ -52,11 +57,14 @@ struct as_file_header *ASParseHeader(unsigned char *buf)
* @param entry_buf The entry buffer
* @return
*/
-struct as_prodos_info *ASParseProdosEntry(unsigned char *entry_buf)
+struct as_prodos_info *ASParseProdosEntry(unsigned char *entry_buf, size_t buflen)
{
- struct as_prodos_info *prodos_entry = malloc(sizeof(as_prodos_info));
- struct as_prodos_info *buf_prodos_entry = (as_prodos_info *) entry_buf;
+ if (buflen < sizeof(as_prodos_info)) return NULL;
+
+ struct as_prodos_info *prodos_entry = calloc(1, sizeof(as_prodos_info));
+ if (!prodos_entry) return NULL;
+ struct as_prodos_info *buf_prodos_entry = (as_prodos_info *) entry_buf;
prodos_entry->access = as_field16(buf_prodos_entry->access);
prodos_entry->filetype = as_field16(buf_prodos_entry->filetype);
prodos_entry->auxtype = as_field32(buf_prodos_entry->auxtype);
@@ -70,7 +78,7 @@ struct as_prodos_info *ASParseProdosEntry(unsigned char *entry_buf)
* @param buf
* @return
*/
-struct as_file_entry *ASGetEntries(struct as_file_header *header, unsigned char *buf)
+struct as_file_entry *ASGetEntries(struct as_file_header *header, unsigned char *buf, size_t buflen)
{
if (!header)
{
@@ -78,12 +86,11 @@ struct as_file_entry *ASGetEntries(struct as_file_header *header, unsigned char
return NULL;
}
- struct as_file_entry *entries = malloc(
- header->num_entries * sizeof(as_file_entry)
- );
+ size_t entries_length = header->num_entries * sizeof(as_file_entry);
+ if (buflen < sizeof(as_file_header) + entries_length) return NULL;
- struct as_file_entry *buf_entries = (as_file_entry *) (buf + sizeof(as_file_header));
- memcpy(entries, buf_entries, header->num_entries * sizeof(as_file_entry));
+ struct as_file_entry *entries = calloc(1, entries_length);
+ memcpy(entries, buf + sizeof(as_file_header), entries_length);
if (IS_LITTLE_ENDIAN)
for (int i = 0; i < header->num_entries; ++i)
@@ -103,16 +110,16 @@ struct as_file_entry *ASGetEntries(struct as_file_header *header, unsigned char
* @param data The data
* @param data_fork_entry The data fork entry
*/
-void ASDecorateDataFork(struct prodos_file *current_file, unsigned char *data, as_file_entry *data_fork_entry)
+void ASDecorateDataFork(struct prodos_file *current_file, unsigned char *data, size_t datalen, as_file_entry *data_fork_entry)
{
if (data_fork_entry->entry_id != data_fork) return;
+ if (datalen < data_fork_entry->offset + data_fork_entry->length) return;
+
unsigned char *data_entry = malloc(data_fork_entry->length);
memcpy(data_entry, data + data_fork_entry->offset, data_fork_entry->length);
current_file->data = data_entry;
current_file->data_length = data_fork_entry->length;
-
- return;
}
/**
@@ -123,21 +130,18 @@ void ASDecorateDataFork(struct prodos_file *current_file, unsigned char *data, a
* @param data The data
* @param prodos_entry The prodos entry
*/
-void ASDecorateProdosFileInfo(struct prodos_file *current_file, unsigned char *data, as_file_entry *prodos_entry)
+void ASDecorateProdosFileInfo(struct prodos_file *current_file, unsigned char *data, size_t datalen, as_file_entry *prodos_entry)
{
if (prodos_entry->entry_id != prodos_file_info) return;
- struct as_prodos_info *info_meta = ASParseProdosEntry(
- data + prodos_entry->offset
- );
+ if (datalen < prodos_entry->offset + prodos_entry->length) return;
+ struct as_prodos_info *info_meta = ASParseProdosEntry(data + prodos_entry->offset, prodos_entry->length);
if (!info_meta) return;
current_file->access = info_meta->access;
current_file->type = info_meta->filetype;
current_file->aux_type = info_meta->auxtype;
-
- return;
}
/**
@@ -147,26 +151,25 @@ void ASDecorateProdosFileInfo(struct prodos_file *current_file, unsigned char *d
* @param current_file
* @param data
*/
-void ASDecorateProdosFile(struct prodos_file *current_file, unsigned char *data)
+void ASDecorateProdosFile(struct prodos_file *current_file, unsigned char *data, size_t datalen)
{
- struct as_file_header *header = ASParseHeader(data);
- struct as_file_entry *entries = ASGetEntries(header, data);
+ struct as_file_header *header = ASParseHeader(data, datalen);
+ struct as_file_entry *entries = ASGetEntries(header, data, datalen);
- for (int i = 0; i < header->num_entries; ++i)
- switch(entries[i].entry_id)
- {
- case data_fork:
- ASDecorateDataFork(current_file, data, &entries[i]);
- break;
- case prodos_file_info:
- ASDecorateProdosFileInfo(current_file, data, &entries[i]);
- break;
- default:
- logf_info(" Entry ID %d unsupported, ignoring!\n", entries[i].entry_id);
- logf_info(" (See https://tools.ietf.org/html/rfc1740 for ID lookup)\n");
- break;
- }
- return;
+ for (int i = 0; i < header->num_entries; ++i)
+ switch(entries[i].entry_id)
+ {
+ case data_fork:
+ ASDecorateDataFork(current_file, data, datalen, &entries[i]);
+ break;
+ case prodos_file_info:
+ ASDecorateProdosFileInfo(current_file, data, datalen, &entries[i]);
+ break;
+ default:
+ logf_info(" Entry ID %d unsupported, ignoring!\n", entries[i].entry_id);
+ logf_info(" (See https://tools.ietf.org/html/rfc1740 for ID lookup)\n");
+ break;
+ }
}
struct as_from_prodos ASFromProdosFile(struct prodos_file *file)
diff --git a/Src/File_AppleSingle.h b/Src/File_AppleSingle.h
index 75edff1..06810d1 100644
--- a/Src/File_AppleSingle.h
+++ b/Src/File_AppleSingle.h
@@ -70,14 +70,14 @@ typedef struct as_from_prodos
#pragma pack(pop)
-bool ASIsAppleSingle(unsigned char *buf);
+bool ASIsAppleSingle(unsigned char *buf, size_t buflen);
-struct as_file_header *ASParseHeader(unsigned char *buf);
-struct as_prodos_info *ASParseProdosEntry(unsigned char *entry_buf);
-struct as_file_entry *ASGetEntries(struct as_file_header *header, unsigned char *buf);
+struct as_file_header *ASParseHeader(unsigned char *bufASDecorateDataFork, size_t buflen);
+struct as_prodos_info *ASParseProdosEntry(unsigned char *entry_buf, size_t buflen);
+struct as_file_entry *ASGetEntries(struct as_file_header *header, unsigned char *buf, size_t buflen);
-void ASDecorateDataFork(struct prodos_file *current_file, unsigned char *data, as_file_entry *data_fork_entry);
-void ASDeocrateProdosFileInfo(struct prodos_file *current_file, unsigned char *data, as_file_entry *prodos_entry);
-void ASDecorateProdosFile(struct prodos_file *current_file, unsigned char *data);
+void ASDecorateDataFork(struct prodos_file *current_file, unsigned char *data, size_t datalen, as_file_entry *data_fork_entry);
+void ASDeocrateProdosFileInfo(struct prodos_file *current_file, unsigned char *data, size_t datalen, as_file_entry *prodos_entry);
+void ASDecorateProdosFile(struct prodos_file *current_file, unsigned char *data, size_t datalen);
struct as_from_prodos ASFromProdosFile(struct prodos_file *file);
diff --git a/Src/Prodos_Add.c b/Src/Prodos_Add.c
index f8c2ca8..5786c06 100644
--- a/Src/Prodos_Add.c
+++ b/Src/Prodos_Add.c
@@ -364,12 +364,12 @@ static struct prodos_file *LoadFile(char *file_path_data, bool zero_case_bits)
return NULL;
}
- bool is_apple_single = ASIsAppleSingle(data);
+ bool is_apple_single = ASIsAppleSingle(data, current_file->data_length);
if (is_apple_single)
{
logf_info(" AppleSingle format detected!\n");
- ASDecorateProdosFile(current_file, data);
+ ASDecorateProdosFile(current_file, data, current_file->data_length);
}
else
{
--
2.24.3 (Apple Git-128)