From 5e4e6d2db892811d269e7aef5055c0e5ed6095fe Mon Sep 17 00:00:00 2001 From: xamidev <121681048+xamidev@users.noreply.github.com> Date: Tue, 10 Sep 2024 20:14:24 +0200 Subject: [PATCH 1/3] Fix: safety: malloc and free BMP image; memcpy sanitizing --- src/kernel/initrd.c | 38 +++++++++++++++++++++++++++++++++++--- src/kernel/initrd.h | 1 + src/kernel/kmain.c | 2 +- src/programs/bmp.c | 9 ++++----- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/kernel/initrd.c b/src/kernel/initrd.c index 70788d6..b1ab2fe 100644 --- a/src/kernel/initrd.c +++ b/src/kernel/initrd.c @@ -131,9 +131,16 @@ int tar_file_to_buffer(uint8_t* initrd, const char* filename, char* buffer) if (strcmp(file_name, filename) == 0) { uint8_t* file_data = current_block + TAR_BLOCK_SIZE; - memcpy(buffer, file_data, file_size); - buffer[file_size] = '\0'; - return 0; + if (sizeof(buffer) >= sizeof(file_data)) + { + memcpy(buffer, file_data, file_size); + buffer[file_size] = '\0'; + return 0; + } else { + printf("Invalid destination buffer size %d bytes < %d bytes\n", sizeof(buffer), sizeof(file_data)); + return -1; + } + return -1; } uint32_t total_size = ((file_size + TAR_BLOCK_SIZE - 1) / TAR_BLOCK_SIZE) * TAR_BLOCK_SIZE; @@ -142,3 +149,28 @@ int tar_file_to_buffer(uint8_t* initrd, const char* filename, char* buffer) printf("[tar] file '%s' not found\n", filename); return -1; } + +uint32_t tar_get_file_size(uint8_t* initrd, const char* filename) +{ + uint8_t* current_block = initrd; + + while (1) + { + if (current_block[0] == '\0') + { + return -1; + } + + const char* file_name = (const char*)current_block; + uint32_t file_size = tar_parse_size((const char*)(current_block+124)); + + if (strcmp(file_name, filename) == 0) + { + return file_size; + } + + uint32_t total_size = ((file_size + TAR_BLOCK_SIZE - 1) / TAR_BLOCK_SIZE) * TAR_BLOCK_SIZE; + current_block += TAR_BLOCK_SIZE + total_size; + } + return -1; +} diff --git a/src/kernel/initrd.h b/src/kernel/initrd.h index 43ffd29..714bf84 100644 --- a/src/kernel/initrd.h +++ b/src/kernel/initrd.h @@ -34,5 +34,6 @@ void tar_find_file(uint8_t *tar_start, const char* filename); void ls_initrd(uint8_t* initrd, int verbose); void cat_initrd(uint8_t* initrd, const char* filename); int tar_file_to_buffer(uint8_t* initrd, const char* filename, char* buffer); +uint32_t tar_get_file_size(uint8_t* initrd, const char* filename); #endif diff --git a/src/kernel/kmain.c b/src/kernel/kmain.c index ac7f6c3..03a9886 100644 --- a/src/kernel/kmain.c +++ b/src/kernel/kmain.c @@ -79,7 +79,7 @@ void kmain(multiboot2_info *mb_info) if (mmap->addr != 0) { - serial_printf(3, "base addr=0x%x%x, length=0x%x%x, type=%u\n", + serial_printf(3, "base addr=0x%x%x, length=0x%x%x, type=%u", (uint32_t) (mmap->addr >> 32), (uint32_t) (mmap->addr & 0xFFFFFFFF), (uint32_t) (mmap->len >> 32), diff --git a/src/programs/bmp.c b/src/programs/bmp.c index 16e8654..95c47b0 100644 --- a/src/programs/bmp.c +++ b/src/programs/bmp.c @@ -9,6 +9,7 @@ #include "../drivers/framebuffer.h" #include "../libc/stdio.h" #include "../drivers/serial.h" +#include "../kernel/kheap.h" #pragma pack(push, 1) typedef struct @@ -38,11 +39,8 @@ typedef struct void display_bmp(uint32_t* fb, int pitch, int bpp, uint8_t* initrd, const char* filename) { - // Should use dynamic allocation when heap works - // Cannot go more than ~500k size for buffer - // Fail zone 450k->470k - // So right now the max should be 400kb img size - char buffer[400*1000]; + uint32_t buf_size = tar_get_file_size(initrd, filename); + char* buffer = (char*)malloc(buf_size); int file_status = tar_file_to_buffer(initrd, filename, buffer); if (file_status != 0) @@ -87,6 +85,7 @@ void display_bmp(uint32_t* fb, int pitch, int bpp, uint8_t* initrd, const char* // Update cursor pos after image drawing move_cursor(get_cursor_x(), get_cursor_y()+(height/16)+2); + free(buffer); } void program_bmp(int argc, char* argv[]) -- 2.49.1 From bf0228d3ac8755e14655788ac34e348fb7e07d12 Mon Sep 17 00:00:00 2001 From: xamidev <121681048+xamidev@users.noreply.github.com> Date: Tue, 10 Sep 2024 20:48:12 +0200 Subject: [PATCH 2/3] Add: strnlen, strncat, safer string functions --- src/libc/string.c | 29 +++++++++++++++++++++++++++++ src/libc/string.h | 4 ++++ 2 files changed, 33 insertions(+) diff --git a/src/libc/string.c b/src/libc/string.c index 91f07f1..fdff71a 100644 --- a/src/libc/string.c +++ b/src/libc/string.c @@ -103,3 +103,32 @@ void strcat(char* dest, const char* src) *dest = '\0'; } + +size_t strnlen(const char* str, size_t max_len) +{ + size_t len = 0; + while (*str && len < max_len) + { + len++; + str++; + } + return len; +} + +void strncat(char* dest, const char* src, size_t n) +{ + while (*dest) + { + dest++; + } + + while (*src && n > 0) + { + *dest = *src; + dest++; + src++; + n--; + } + + *dest = '\0'; +} diff --git a/src/libc/string.h b/src/libc/string.h index 590d29a..509f5ba 100644 --- a/src/libc/string.h +++ b/src/libc/string.h @@ -12,4 +12,8 @@ char* strtok(char* str, const char* delimiter); int atoi(char* str); void strcat(char* dest, const char* src); +// Safer functions +size_t strnlen(const char* str, size_t max_len); +void strncat(char* dest, const char* src, size_t n); + #endif -- 2.49.1 From 64ccec0866b52b4114a7e3e085431c275c88558f Mon Sep 17 00:00:00 2001 From: xamidev <121681048+xamidev@users.noreply.github.com> Date: Tue, 10 Sep 2024 21:11:41 +0200 Subject: [PATCH 3/3] Fix: explicit typecasting --- src/initrd/stuff/nice.txt | 8 ++++++++ src/kernel/gdt.c | 6 +++--- src/kernel/idt.c | 2 +- src/kernel/initrd.c | 2 +- src/kernel/kmain.c | 6 +++--- src/libc/string.h | 2 ++ src/programs/bmp.c | 2 +- src/programs/navalbattle.c | 4 ++-- src/programs/primes.c | 2 +- 9 files changed, 22 insertions(+), 12 deletions(-) create mode 100644 src/initrd/stuff/nice.txt diff --git a/src/initrd/stuff/nice.txt b/src/initrd/stuff/nice.txt new file mode 100644 index 0000000..5849978 --- /dev/null +++ b/src/initrd/stuff/nice.txt @@ -0,0 +1,8 @@ +Subfolder support! + +I am making a bit of scurity improvements but clearly it has no meaing here. +The real meanng is the tellin of the Genesis; a true work of art. So many +cool stuff here!! Look, there are functions, comands, bits and bytes, conditions, +and lgorithms. What a fantastic world! But after all it stays formless and empty. + +1:5:1 1:1:1 7:8:1 1:7:3 1:4:3 2:1:2 diff --git a/src/kernel/gdt.c b/src/kernel/gdt.c index e87a980..bb9fddd 100644 --- a/src/kernel/gdt.c +++ b/src/kernel/gdt.c @@ -36,9 +36,9 @@ void gdt_install() gdt_set_gate(2, 0, 0xFFFFFFFF, 0x92, 0xCF); // Ring 3 - gdt_set_gate(3, 0, 0xFFFFFFFF, 0xFA, 0xCF); - gdt_set_gate(4, 0, 0xFFFFFFFF, 0xF2, 0xCF); + //gdt_set_gate(3, 0, 0xFFFFFFFF, 0xFA, 0xCF); + //gdt_set_gate(4, 0, 0xFFFFFFFF, 0xF2, 0xCF); gdt_flush(); - printf("[kernel] GDT gates set (ring 0 and 3), gdt=0x%x\n", &gdt); + printf("[kernel] GDT gates set (ring 0), gdt=0x%x\n", (unsigned int)&gdt); } diff --git a/src/kernel/idt.c b/src/kernel/idt.c index f859ec4..e5f6f4b 100644 --- a/src/kernel/idt.c +++ b/src/kernel/idt.c @@ -30,5 +30,5 @@ void idt_install() memset(&idt, 0, sizeof(struct idt_entry)*256); idt_load(); - printf("[kernel] loaded IDT at idt=0x%x\n", &idt); + printf("[kernel] loaded IDT at idt=0x%x\n", (unsigned int)&idt); } diff --git a/src/kernel/initrd.c b/src/kernel/initrd.c index b1ab2fe..0742d4f 100644 --- a/src/kernel/initrd.c +++ b/src/kernel/initrd.c @@ -82,7 +82,7 @@ void ls_initrd(uint8_t* initrd, int verbose) { printf("%s\n", header->filename); } else { - printf("%7d\t%c\t %s\n", header->size, header->typeflag, header->filename); + printf("%7d\t%c\t %s\n", (int)header->size, header->typeflag, header->filename); } uint32_t size = tar_parse_size(header->size); diff --git a/src/kernel/kmain.c b/src/kernel/kmain.c index 03a9886..26b0367 100644 --- a/src/kernel/kmain.c +++ b/src/kernel/kmain.c @@ -65,12 +65,12 @@ void kmain(multiboot2_info *mb_info) } printf("[kernel] multiboot2 info at 0x%x, size=%u\n", mb_info, mb_info->total_size); - printf("[kernel] framebuffer discovered at 0x%x\n", fb_info->framebuffer_addr); + printf("[kernel] framebuffer discovered at 0x%x\n", (unsigned int)fb_info->framebuffer_addr); printf("[kernel] fb0: width=%u, height=%u, pitch=%u, bpp=%u\n", fb_info->framebuffer_width, fb_info->framebuffer_height, fb_info->framebuffer_pitch, fb_info->framebuffer_bpp); if (mmap_tag) // memmap debug print { - printf("[kernel] found memory map tag by multiboot2\n"); + puts("[kernel] found memory map tag by multiboot2\n"); struct multiboot_mmap_entry *mmap = mmap_tag->entries; while ((uint8_t*) mmap < tags + mmap_tag->size) @@ -115,7 +115,7 @@ void kmain(multiboot2_info *mb_info) init_alloc(); void* ptr1 = malloc(256); void* ptr2 = malloc(512); - printf("[debug] malloc test ptr1=0x%x, ptr2=0x%x\n", ptr1, ptr2); + printf("[debug] malloc test ptr1=0x%x, ptr2=0x%x\n", (unsigned int)ptr1, (unsigned int)ptr2); free(ptr1); free(ptr2); timer_install(); diff --git a/src/libc/string.h b/src/libc/string.h index 509f5ba..f5a77fc 100644 --- a/src/libc/string.h +++ b/src/libc/string.h @@ -6,6 +6,8 @@ #ifndef STRING_H #define STRING_H +#include "../kernel/system.h" + int strlen(const char* str); int strcmp(const char* str1, const char* str2); char* strtok(char* str, const char* delimiter); diff --git a/src/programs/bmp.c b/src/programs/bmp.c index 95c47b0..f813ce4 100644 --- a/src/programs/bmp.c +++ b/src/programs/bmp.c @@ -62,7 +62,7 @@ void display_bmp(uint32_t* fb, int pitch, int bpp, uint8_t* initrd, const char* int height = bmp_info->biHeight; int pixel_offset = bmp_header->bfOffBits; - printf("%d-bit BMP, width: %d, height: %d, pixel offset: %d\n", bmp_info->biBitCount, bmp_info->biWidth, bmp_info->biHeight, bmp_header->bfOffBits); + printf("%d-bit BMP, width: %d, height: %d, pixel offset: %d\n", bmp_info->biBitCount, bmp_info->biWidth, bmp_info->biHeight, (int)bmp_header->bfOffBits); erase_cursor(); uint8_t* pixel_data = (uint8_t*)(buffer + pixel_offset); diff --git a/src/programs/navalbattle.c b/src/programs/navalbattle.c index 8f1bfb5..dbc50fb 100644 --- a/src/programs/navalbattle.c +++ b/src/programs/navalbattle.c @@ -129,7 +129,7 @@ void placing_ally_ships(grid_t* grid[SIZE][SIZE]) do { - printf("Ship %d\n------\n", i); + printf("Ship %d\n------\n", (int)i); puts("X coord: "); char input_buffer[BUFFER_SIZE]; get_input(input_buffer, BUFFER_SIZE); @@ -179,7 +179,7 @@ void show_enemy_battlefield(grid_t* grid[SIZE][SIZE]) for (size_t i=0; i