From b3d1399237c18f54deea80f7d3dd8911514d7ddb Mon Sep 17 00:00:00 2001 From: Matt Mascarenhas Date: Sat, 29 Jul 2017 01:36:14 +0100 Subject: [PATCH] hmml_to_html.c: Prevent buffer overflow errors * REF_MAX_IDENTIFIER sanity (i.e. check that we're below the max before adding a new one) * Make ClaimBuffer() check that there's enough space left in the arena --- hmml_to_html/hmml_to_html.c | 126 ++++++++++++++++++++++-------------- 1 file changed, 76 insertions(+), 50 deletions(-) diff --git a/hmml_to_html/hmml_to_html.c b/hmml_to_html/hmml_to_html.c index 62c7b13..524f4f5 100644 --- a/hmml_to_html/hmml_to_html.c +++ b/hmml_to_html/hmml_to_html.c @@ -51,7 +51,7 @@ typedef struct int Identifier; } identifier; -#define REF_MAX_IDENTIFIER 32 +#define REF_MAX_IDENTIFIER 55 typedef struct { @@ -84,10 +84,28 @@ char *Credentials[ ][5] = #define ArrayCount(A) sizeof(A)/sizeof(*(A)) +#define ClaimBuffer(MemoryArena, ClaimedMemory, Buffer, ID, Size) if(__ClaimBuffer(MemoryArena, ClaimedMemory, Buffer, ID, Size))\ +{\ + fprintf(stderr, "%s:%d: MemoryArena cannot contain %s of size %d\n", __FILE__, __LINE__, ID, Size);\ + hmml_free(&HMML);\ + FreeBuffer(MemoryArena);\ + return 1;\ +}; + void -ClaimBuffer(char *MemoryArena, int *ClaimedMemory, buffer *Buffer, char *ID, int Size) +FreeBuffer(buffer *Buffer) { - Buffer->Location = MemoryArena + *ClaimedMemory; + free(Buffer->Location); +} + +int +__ClaimBuffer(buffer *MemoryArena, int *ClaimedMemory, buffer *Buffer, char *ID, int Size) +{ + if(*ClaimedMemory + Size > MemoryArena->Size) + { + return 1; + } + Buffer->Location = MemoryArena->Location + *ClaimedMemory; Buffer->Size = Size; Buffer->ID = ID; *ClaimedMemory += Buffer->Size; @@ -97,6 +115,7 @@ ClaimBuffer(char *MemoryArena, int *ClaimedMemory, buffer *Buffer, char *ID, int printf(" Claimed: %s: %d\n" " Total ClaimedMemory: %d\n\n", Buffer->ID, Buffer->Size, *ClaimedMemory); #endif + return 0; } void @@ -1287,9 +1306,9 @@ main(int ArgC, char **Args) } // NOTE(matt): Init MemoryArena - char *MemoryArena; - int ArenaSize = Megabytes(4); - if(!(MemoryArena = calloc(ArenaSize, 1))) + buffer MemoryArena; + MemoryArena.Size = Megabytes(4); + if(!(MemoryArena.Location = calloc(MemoryArena.Size, 1))) { perror(Args[0]); return 1; @@ -1354,17 +1373,17 @@ main(int ArgC, char **Args) if(!(InFile = fopen(Args[FileIndex], "r"))) { perror(Args[FileIndex]); - free(MemoryArena); + free(MemoryArena.Location); return 1; } + HMML_Output HMML = hmml_parse_file(InFile); + fclose(InFile); + #if CONFIG ClaimBuffer(MemoryArena, &ClaimedMemory, &Config, "Config", Kilobytes(1)); #endif - HMML_Output HMML = hmml_parse_file(InFile); - fclose(InFile); - if(HMML.well_formed) { #if DEBUG @@ -1390,25 +1409,25 @@ main(int ArgC, char **Args) // Script // FilterState - ClaimBuffer(MemoryArena, &ClaimedMemory, &Master, "Master", Kilobytes(512)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &Includes, "Includes", Kilobytes(1)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &Master, "Master", Kilobytes(512)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &Includes, "Includes", Kilobytes(1)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &Menus, "Menus", Kilobytes(24)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &QuoteMenu, "QuoteMenu", Kilobytes(16)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &ReferenceMenu, "ReferenceMenu", Kilobytes(16)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &FilterMenu, "FilterMenu", Kilobytes(16)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &FilterTopics, "FilterTopics", Kilobytes(8)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &FilterMedia, "FilterMedia", Kilobytes(8)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &CreditsMenu, "CreditsMenu", Kilobytes(8)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &HostInfo, "HostInfo", Kilobytes(1)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &AnnotatorInfo, "AnnotatorInfo", Kilobytes(1)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &Menus, "Menus", Kilobytes(24)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &QuoteMenu, "QuoteMenu", Kilobytes(16)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &ReferenceMenu, "ReferenceMenu", Kilobytes(16)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &FilterMenu, "FilterMenu", Kilobytes(16)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &FilterTopics, "FilterTopics", Kilobytes(8)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &FilterMedia, "FilterMedia", Kilobytes(8)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &CreditsMenu, "CreditsMenu", Kilobytes(8)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &HostInfo, "HostInfo", Kilobytes(1)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &AnnotatorInfo, "AnnotatorInfo", Kilobytes(1)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &Player, "Player", Kilobytes(256)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &Colour, "Colour", 32); - ClaimBuffer(MemoryArena, &ClaimedMemory, &Annotation, "Annotation", Kilobytes(8)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &Player, "Player", Kilobytes(256)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &Colour, "Colour", 32); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &Annotation, "Annotation", Kilobytes(8)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &Script, "Script", Kilobytes(8)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &FilterState, "FilterState", Kilobytes(4)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &Script, "Script", Kilobytes(8)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &FilterState, "FilterState", Kilobytes(4)); ref_info ReferencesArray[200] = { 0 }; category_info TopicsArray[56] = { 0 }; @@ -1446,7 +1465,7 @@ main(int ArgC, char **Args) { fprintf(stderr, "%s: Missing annotator in [video] node\n", Args[FileIndex]); hmml_free(&HMML); - free(MemoryArena); + free(MemoryArena.Location); return 1; } @@ -1474,11 +1493,11 @@ main(int ArgC, char **Args) // Text // Category - ClaimBuffer(MemoryArena, &ClaimedMemory, &AnnotationHeader, "AnnotationHeader", 512); - ClaimBuffer(MemoryArena, &ClaimedMemory, &AnnotationClass, "AnnotationClass", 256); - ClaimBuffer(MemoryArena, &ClaimedMemory, &AnnotationData, "AnnotationData", 256); - ClaimBuffer(MemoryArena, &ClaimedMemory, &Text, "Text", Kilobytes(4)); - ClaimBuffer(MemoryArena, &ClaimedMemory, &Category, "Category", 512); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &AnnotationHeader, "AnnotationHeader", 512); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &AnnotationClass, "AnnotationClass", 256); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &AnnotationData, "AnnotationData", 256); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &Text, "Text", Kilobytes(4)); + ClaimBuffer(&MemoryArena, &ClaimedMemory, &Category, "Category", 512); CopyStringToBuffer(&AnnotationHeader, "
line); hmml_free(&HMML); - free(MemoryArena); + free(MemoryArena.Location); return 1; } ++ReferencesArray[RefIdentifier - 1].IdentifierCount; @@ -1628,6 +1647,13 @@ main(int ArgC, char **Args) { for(int i = 0; i < UniqueRefs; ++i) { + if(ReferencesArray[i].IdentifierCount == REF_MAX_IDENTIFIER) + { + fprintf(stderr, "%s:%d: Too many timecodes associated with one reference (increase REF_MAX_IDENTIFIER)\n", Args[FileIndex], Anno->line); + hmml_free(&HMML); + free(MemoryArena.Location); + return 1; + } if(CurrentRef->isbn) { if(!StringsDiffer(CurrentRef->isbn, ReferencesArray[i].ID)) @@ -1652,7 +1678,7 @@ main(int ArgC, char **Args) { fprintf(stderr, "%s:%d: Reference must have an ISBN or URL\n", Args[FileIndex], Anno->line); hmml_free(&HMML); - free(MemoryArena); + free(MemoryArena.Location); return 1; } } @@ -1661,7 +1687,7 @@ main(int ArgC, char **Args) { fprintf(stderr, "%s:%d: Cannot process new combination of reference info\n", Args[FileIndex], Anno->line); hmml_free(&HMML); - free(MemoryArena); + free(MemoryArena.Location); return 1; } ++ReferencesArray[UniqueRefs].IdentifierCount; @@ -1682,7 +1708,7 @@ main(int ArgC, char **Args) { fprintf(stderr, "%s:%d: Reference must have an ISBN or URL\n", Args[FileIndex], Anno->line); hmml_free(&HMML); - free(MemoryArena); + free(MemoryArena.Location); return 1; } @@ -1702,7 +1728,7 @@ main(int ArgC, char **Args) { fprintf(stderr, "%s:%d: Reference must have an ISBN or URL", Args[FileIndex], Anno->line); hmml_free(&HMML); - free(MemoryArena); + free(MemoryArena.Location); return 1; } } @@ -1784,7 +1810,7 @@ main(int ArgC, char **Args) HMML.metadata.stream_username ? HMML.metadata.stream_username : HMML.metadata.member, Anno->quote.id); hmml_free(&HMML); - free(MemoryArena); + free(MemoryArena.Location); return 1; } @@ -2516,7 +2542,7 @@ main(int ArgC, char **Args) FILE *TemplateFile; if(!(TemplateFile = fopen(TemplateLocation, "r"))) { - perror(Args[0]); hmml_free(&HMML); free(MemoryArena); return 1; + perror(Args[0]); hmml_free(&HMML); free(MemoryArena.Location); return 1; } fseek(TemplateFile, 0, SEEK_END); @@ -2526,7 +2552,7 @@ main(int ArgC, char **Args) fseek(TemplateFile, 0, SEEK_SET); if(!(Template.Location = malloc(Template.Size))) { - perror(Args[0]); hmml_free(&HMML); free(MemoryArena); return 1; + perror(Args[0]); hmml_free(&HMML); free(MemoryArena.Location); return 1; } Template.Ptr = Template.Location; fread(Template.Location, Template.Size, 1, TemplateFile); @@ -2537,7 +2563,7 @@ main(int ArgC, char **Args) Output.ID = "Output"; if(!(Output.Location = malloc(Output.Size))) { - perror(Args[0]); free(Template.Location); hmml_free(&HMML); free(MemoryArena); return 1; + perror(Args[0]); free(Template.Location); hmml_free(&HMML); free(MemoryArena.Location); return 1; } Output.Ptr = Output.Location; @@ -2564,7 +2590,7 @@ main(int ArgC, char **Args) if(FoundIncludes == TRUE) { fprintf(stderr, "Template contains more than one tag\n"); - free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena); return 1; + free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena.Location); return 1; } FoundIncludes = TRUE; Output.Ptr = CommentStart; @@ -2601,7 +2627,7 @@ main(int ArgC, char **Args) if(FoundMenus == TRUE) { fprintf(stderr, "Template contains more than one tag\n"); - free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena); return 1; + free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena.Location); return 1; } FoundMenus = TRUE; Output.Ptr = CommentStart; @@ -2622,7 +2648,7 @@ main(int ArgC, char **Args) if(FoundPlayer == TRUE) { fprintf(stderr, "Template contains more than one tag\n"); - free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena); return 1; + free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena.Location); return 1; } FoundPlayer = TRUE; Output.Ptr = CommentStart; @@ -2643,12 +2669,12 @@ main(int ArgC, char **Args) if(FoundPlayer == FALSE) { fprintf(stderr, " must come after \n"); - free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena); return 1; + free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena.Location); return 1; } if(FoundScript == TRUE) { fprintf(stderr, "Template contains more than one tag\n"); - free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena); return 1; + free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena.Location); return 1; } FoundScript = TRUE; Output.Ptr = CommentStart; @@ -2682,7 +2708,7 @@ main(int ArgC, char **Args) FILE *OutFile; if(!(OutFile = fopen(OutIntegratedLocation, "w"))) { - perror(OutIntegratedLocation); free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena); return 1; + perror(OutIntegratedLocation); free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena.Location); return 1; } fwrite(Output.Location, Output.Ptr - Output.Location, 1, OutFile); fclose(OutFile); @@ -2694,7 +2720,7 @@ main(int ArgC, char **Args) if(!FoundMenus) { fprintf(stderr, " \n"); } if(!FoundPlayer) { fprintf(stderr, " \n"); } if(!FoundScript) { fprintf(stderr, " \n"); } - free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena); return 1; + free(Template.Location); free(Output.Location); hmml_free(&HMML); free(MemoryArena.Location); return 1; } free(Template.Location); @@ -2728,7 +2754,7 @@ main(int ArgC, char **Args) FILE *OutFile; if(!(OutFile = fopen(OutLocation, "w"))) { - perror(OutLocation); hmml_free(&HMML); free(MemoryArena); return 1; + perror(OutLocation); hmml_free(&HMML); free(MemoryArena.Location); return 1; } fwrite(Master.Location, Master.Ptr - Master.Location, 1, OutFile); fclose(OutFile); @@ -2777,5 +2803,5 @@ main(int ArgC, char **Args) } hmml_free(&HMML); } - free(MemoryArena); + free(MemoryArena.Location); }