diff --git a/BaseTools/Source/C/BrotliCompress/BrotliCompress.c b/BaseTools/Source/C/BrotliCompress/BrotliCompress.c index c6b018905a..4a6db1a2cd 100644 --- a/BaseTools/Source/C/BrotliCompress/BrotliCompress.c +++ b/BaseTools/Source/C/BrotliCompress/BrotliCompress.c @@ -407,8 +407,6 @@ int main(int argc, char** argv) { FILE *OutputHandle; int Quality; int Gap; - int OutputFileLength; - int InputFileLength; int Ret; size_t InputFileSize; uint8_t *Buffer; @@ -440,87 +438,71 @@ int main(int argc, char** argv) { Version(); return 0; } - while (argc > 1) { - if (strcmp(argv[1], "-e") == 0 || strcmp(argv[1], "--compress") == 0 ) { + int i = 1; + while (i < argc) { + const char* arg = argv[i]; + if (strcmp(arg, "-e") == 0 || strcmp(arg, "--compress") == 0) { CompressBool = BROTLI_TRUE; if (DecompressBool) { - printf("Can't use -e/--compress with -d/--decompess on the same time\n"); + fprintf(stderr, "Can't use -e/--compress with -d/--decompress at the same time\n"); return 1; } - argc--; - argv++; + i++; continue; } - if (strcmp(argv[1], "-d") == 0 || strcmp(argv[1], "--decompress") == 0 ) { + if (strcmp(arg, "-d") == 0 || strcmp(arg, "--decompress") == 0) { DecompressBool = BROTLI_TRUE; if (CompressBool) { - printf("Can't use -e/--compress with -d/--decompess on the same time\n"); + fprintf(stderr, "Can't use -e/--compress with -d/--decompress at the same time\n"); return 1; } - argc--; - argv++; + i++; continue; } - if (strcmp(argv[1], "-o") == 0 || strncmp(argv[1], "--output", 8) == 0) { - if (strcmp(argv[1], "-o") == 0) { - OutputFileLength = strlen(argv[2]); - if (OutputFileLength > _MAX_PATH) { - printf ("The file path %s is too long\n", argv[2]); - return 1; - } - OutputFile = argv[2]; - if (OutputFile == NULL) { - fprintf(stderr, "Input file can't be null\n"); - return 1; - } - argc--; - argv++; - } else { - OutputFileLength = strlen(argv[1] - 9); - OutputFile = (char *)argv[1] + 9; - } - argc--; - argv++; - continue; - } - if (strcmp(argv[1], "-q") == 0 || strncmp(argv[1], "--quality", 9) == 0) { - if (strcmp(argv[1], "-q") == 0) { - Quality = strtol(argv[2], NULL, 16); - argc--; - argv++; - } else { - Quality = strtol((char *)argv[1] + 10, NULL, 16); - } - argc--; - argv++; - continue; - } - if (strcmp(argv[1], "-g") == 0 || strncmp(argv[1], "--gap", 5) == 0) { - if (strcmp(argv[1], "-g") == 0) { - Gap = strtol(argv[2], NULL, 16); - argc--; - argv++; - } else { - Gap = strtol((char *)argv[1] + 6, NULL, 16); - } - argc--; - argv++; - continue; - } - if (argc > 1) { - InputFileLength = strlen(argv[1]); - if (InputFileLength > _MAX_PATH - 1) { - printf ("The file path %s is too long\n", argv[2]); + if (strcmp(arg, "-o") == 0) { + if (i + 1 >= argc) { + fprintf(stderr, "-o requires an argument\n"); return 1; } - InputFile = argv[1]; - if (InputFile == NULL) { - printf("Input file can't be null\n"); - return 1; - } - argc--; - argv++; + OutputFile = argv[i + 1]; + i += 2; + continue; } + if (strncmp(arg, "--output=", 9) == 0) { + OutputFile = (char*)arg + 9; + i++; + continue; + } + if (strcmp(arg, "-q") == 0) { + if (i + 1 >= argc) { + fprintf(stderr, "-q requires an argument\n"); + return 1; + } + Quality = strtol(argv[i + 1], NULL, 0); + i += 2; + continue; + } + if (strncmp(arg, "--quality=", 10) == 0) { + Quality = strtol(arg + 10, NULL, 0); + i++; + continue; + } + if (strcmp(arg, "-g") == 0) { + if (i + 1 >= argc) { + fprintf(stderr, "-g requires an argument\n"); + return 1; + } + Gap = strtol(argv[i + 1], NULL, 0); + i += 2; + continue; + } + if (strncmp(arg, "--gap=", 6) == 0) { + Gap = strtol(arg + 6, NULL, 0); + i++; + continue; + } + InputFile = argv[i]; + i++; } Buffer = (uint8_t*)malloc(kFileBufferSize * 2); @@ -543,14 +525,12 @@ int main(int argc, char** argv) { // // Decompress file for get Outputfile size // - strcpy (OutputTmpFile, OutputFile); - if (strlen(InputFile) + strlen(".tmp") < _MAX_PATH) { - strcat(OutputTmpFile, ".tmp"); - } else { + if (strlen(OutputFile) + strlen(".tmp") >= _MAX_PATH) { printf ("Output file path is too long[%s]\n", OutputFile); Ret = BROTLI_FALSE; goto Finish; } + snprintf(OutputTmpFile, _MAX_PATH, "%s.tmp", OutputFile); memset(Buffer, 0, kFileBufferSize*2); Ret = DecompressFile(OutputFile, InputBuffer, OutputTmpFile, OutputBuffer, Quality, Gap); if (!Ret) { diff --git a/BaseTools/Source/C/Common/EfiUtilityMsgs.c b/BaseTools/Source/C/Common/EfiUtilityMsgs.c index faf354064c..80e1f3d20e 100644 --- a/BaseTools/Source/C/Common/EfiUtilityMsgs.c +++ b/BaseTools/Source/C/Common/EfiUtilityMsgs.c @@ -403,12 +403,12 @@ PrintMessage ( ); } if (Cptr != NULL) { - strcpy (Line, ": "); - strncat (Line, Cptr, MAX_LINE_LEN - strlen (Line) - 1); if (LineNumber != 0) { - sprintf (Line2, "(%u)", (unsigned) LineNumber); - strncat (Line, Line2, MAX_LINE_LEN - strlen (Line) - 1); + snprintf(Line, MAX_LINE_LEN, ": %s(%u)", Cptr, (unsigned) LineNumber); + } else { + snprintf(Line, MAX_LINE_LEN, ": %s", Cptr); } + } } else { // @@ -421,7 +421,8 @@ PrintMessage ( strncpy (Line, Cptr, MAX_LINE_LEN - 1); Line[MAX_LINE_LEN - 1] = 0; if (LineNumber != 0) { - sprintf (Line2, "(%u)", (unsigned) LineNumber); + snprintf (Line2, MAX_LINE_LEN, "(%u)", (unsigned) LineNumber); + strncat (Line, Line2, MAX_LINE_LEN - strlen (Line) - 1); } } else { @@ -448,7 +449,8 @@ PrintMessage ( strncat (Line, ": ", MAX_LINE_LEN - strlen (Line) - 1); strncat (Line, Type, MAX_LINE_LEN - strlen (Line) - 1); if (MessageCode != 0) { - sprintf (Line2, " %04u", (unsigned) MessageCode); + snprintf (Line2, MAX_LINE_LEN, " %04u", (unsigned) MessageCode); + strncat (Line, Line2, MAX_LINE_LEN - strlen (Line) - 1); } fprintf (stdout, "%s", Line); @@ -464,7 +466,8 @@ PrintMessage ( // Print formatted message if provided // if (MsgFmt != NULL) { - vsprintf (Line2, MsgFmt, List); + vsnprintf (Line2, MAX_LINE_LEN, MsgFmt, List); + fprintf (stdout, " %s\n", Line2); } @@ -489,7 +492,8 @@ PrintSimpleMessage ( // Print formatted message if provided // if (MsgFmt != NULL) { - vsprintf (Line, MsgFmt, List); + vsnprintf (Line, MAX_LINE_LEN, MsgFmt, List); + fprintf (stdout, "%s\n", Line); } } diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c index 83f1b285c2..ec809ac4a9 100644 --- a/BaseTools/Source/C/GenFw/Elf64Convert.c +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c @@ -390,7 +390,8 @@ FindPrmHandler ( PrmHandler = (PRM_HANDLER_EXPORT_DESCRIPTOR_STRUCT *)(PrmExport + 1); for (HandlerNum = 0; HandlerNum < PrmExport->NumberPrmHandlers; HandlerNum++) { - strcpy(mExportSymName[mExportSymNum], PrmHandler->PrmHandlerName); + snprintf(mExportSymName[mExportSymNum], PRM_HANDLER_NAME_MAXIMUM_LENGTH, "%s", PrmHandler->PrmHandlerName); + mExportSymNum ++; PrmHandler += 1; @@ -1057,7 +1058,8 @@ ScanSections64 ( // FindPrmHandler(Sym->st_value); - strcpy(mExportSymName[mExportSymNum], (CHAR8*)SymName); + snprintf(mExportSymName[mExportSymNum], PRM_HANDLER_NAME_MAXIMUM_LENGTH, "%s", (CHAR8*)SymName); + mExportRVA[mExportSymNum] = (UINT32)(Sym->st_value); mExportSize += 2 * EFI_IMAGE_EXPORT_ADDR_SIZE + EFI_IMAGE_EXPORT_ORDINAL_SIZE + strlen((CHAR8 *)SymName) + 1; mExportSymNum ++; diff --git a/BaseTools/Source/C/VfrCompile/Pccts/dlg/dlg_a.c b/BaseTools/Source/C/VfrCompile/Pccts/dlg/dlg_a.c index 0b8982cf2a..d550b2a3fa 100644 --- a/BaseTools/Source/C/VfrCompile/Pccts/dlg/dlg_a.c +++ b/BaseTools/Source/C/VfrCompile/Pccts/dlg/dlg_a.c @@ -56,8 +56,8 @@ int lexMember = 0; /* <<%%lexmemeber ...>> MR1 */ int lexAction = 0; /* <<%%lexaction ...>> MR1 */ int parserClass = 0; /* <<%%parserclass ...>> MR1 */ int lexPrefix = 0; /* <<%%lexprefix ...>> MR1 */ -char theClassName[100]; /* MR11 */ -char *pClassName=theClassName; /* MR11 */ +char theClassName[128]; /* MR11 */ +size_t class_len=0; int firstLexMember=1; /* MR1 */ #ifdef __USE_PROTOS @@ -67,9 +67,11 @@ void xxputc(int c) { /* MR1 */ int c; /* MR1 */ { /* MR1 */ #endif - if (parserClass) { /* MR1 */ - *pClassName++=c; /* MR1 */ - *pClassName=0; /* MR1 */ + if (parserClass) { + if (class_len < sizeof(theClassName) - 1) { + theClassName[class_len++] = c; + theClassName[class_len] = '\0'; + } } else if (lexMember || lexPrefix) { /* MR1 */ if (class_stream != NULL) fputc(c,class_stream); /* MR1 */ } else { /* MR1 */ @@ -155,6 +157,8 @@ static void act9() { NLA = PARSERCLASS; parserClass=1; /* MR1 */ + class_len = 0; + theClassName[0] = '\0'; zzmode(ACT); /* MR1 */ } diff --git a/BaseTools/Source/C/VfrCompile/Pccts/dlg/output.c b/BaseTools/Source/C/VfrCompile/Pccts/dlg/output.c index 2e56a6d8c1..3cd64af2f2 100644 --- a/BaseTools/Source/C/VfrCompile/Pccts/dlg/output.c +++ b/BaseTools/Source/C/VfrCompile/Pccts/dlg/output.c @@ -122,7 +122,8 @@ char *name; #endif { static char buf[100]; - sprintf(buf, "%s_h", name); + snprintf(buf, sizeof(buf), "%s_h", name); + return buf; } @@ -712,7 +713,8 @@ char *suffix; static char buf[200]; extern char *class_name; - sprintf(buf, "%s%s", class_name, suffix); + snprintf(buf, sizeof(buf), "%s%s", class_name, suffix); + return buf; } diff --git a/BaseTools/Source/C/VfrCompile/Pccts/dlg/support.c b/BaseTools/Source/C/VfrCompile/Pccts/dlg/support.c index 84fe99d69c..3a2073eb42 100644 --- a/BaseTools/Source/C/VfrCompile/Pccts/dlg/support.c +++ b/BaseTools/Source/C/VfrCompile/Pccts/dlg/support.c @@ -227,14 +227,13 @@ char *n; p = n; /* Copy new output directory into newname[] */ - strcpy(newname, OutputDirectory); - - /* if new output directory does not have trailing dir_sym, add it! */ - if (newname[strlen(newname)-1] != *dir_sym) - strcat(newname, dir_sym); - - /* contatenate FILE NAME ONLY to new output directory */ - strcat(newname, p); + if (snprintf(newname, sizeof(newname), "%s%s%s", + OutputDirectory, + (OutputDirectory[strlen(OutputDirectory)-1] == *dir_sym ? "" : dir_sym), + p) >= (int)sizeof(newname)) { + fprintf(stderr, "dlg: warning: output directory or filename is too long\n"); + return n; /* Return original name if path is too long */ + } return newname; } diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp index 5f4d262d85..0c5355cb69 100644 --- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp +++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp @@ -594,9 +594,13 @@ CVfrCompiler::PreProcess ( VOID ) { - FILE *pVfrFile = NULL; UINT32 CmdLen = 0; CHAR8 *PreProcessCmd = NULL; + FILE *pVfrFile = NULL; + FILE *pPipe = NULL; + FILE *pOutFile = NULL; + int RetVal = -1; + char ReadBuf[256]; if (!IS_RUN_STATUS(STATUS_INITIALIZED)) { goto Fail; @@ -612,8 +616,9 @@ CVfrCompiler::PreProcess ( } fclose (pVfrFile); + // Safely calculate command length CmdLen = strlen (mPreProcessCmd) + strlen (mPreProcessOpt) + - strlen (mOptions.VfrFileName) + strlen (mOptions.PreprocessorOutputFileName); + strlen (mOptions.VfrFileName) + 20; // Extra space for spaces and null terminator if (mOptions.CPreprocessorOptions != NULL) { CmdLen += strlen (mOptions.CPreprocessorOptions); } @@ -621,38 +626,82 @@ CVfrCompiler::PreProcess ( CmdLen += strlen (mOptions.IncludePaths); } - PreProcessCmd = new CHAR8[CmdLen + 10]; + PreProcessCmd = new CHAR8[CmdLen]; if (PreProcessCmd == NULL) { DebugError (NULL, 0, 4001, "Resource: memory can't be allocated", NULL); goto Fail; } - strcpy (PreProcessCmd, mPreProcessCmd), strcat (PreProcessCmd, " "); - strcat (PreProcessCmd, mPreProcessOpt), strcat (PreProcessCmd, " "); - if (mOptions.IncludePaths != NULL) { - strcat (PreProcessCmd, mOptions.IncludePaths), strcat (PreProcessCmd, " "); - } - if (mOptions.CPreprocessorOptions != NULL) { - strcat (PreProcessCmd, mOptions.CPreprocessorOptions), strcat (PreProcessCmd, " "); - } - strcat (PreProcessCmd, mOptions.VfrFileName), strcat (PreProcessCmd, " > "); - strcat (PreProcessCmd, mOptions.PreprocessorOutputFileName); - if (system (PreProcessCmd) != 0) { + // Safely build command string for popen + // Quote VfrFileName to mitigate command injection + snprintf(PreProcessCmd, CmdLen, "%s %s %s %s \"%s\"", + mPreProcessCmd, + mPreProcessOpt, + mOptions.IncludePaths ? mOptions.IncludePaths : "", + mOptions.CPreprocessorOptions ? mOptions.CPreprocessorOptions : "", + mOptions.VfrFileName); + + // + // Execute the command via a pipe and redirect its output to the destination file. + // Using popen allows capturing output. We quote the filename to mitigate + // command injection, although popen still invokes a shell. + // + #ifdef _WIN32 + pPipe = _popen(PreProcessCmd, "r"); + #else + pPipe = popen(PreProcessCmd, "r"); + #endif + + if (pPipe == NULL) { DebugError (NULL, 0, 0003, "Error parsing file", "failed to spawn C preprocessor on VFR file %s\n", PreProcessCmd); goto Fail; } - delete[] PreProcessCmd; + pOutFile = fopen(mOptions.PreprocessorOutputFileName, "w"); + if (pOutFile == NULL) { + DebugError(NULL, 0, 0001, "Error opening output file for writing", mOptions.PreprocessorOutputFileName); + #ifdef _WIN32 + _pclose(pPipe); + #else + pclose(pPipe); + #endif + goto Fail; + } + + while (fgets(ReadBuf, sizeof(ReadBuf), pPipe) != NULL) { + fputs(ReadBuf, pOutFile); + } + + fclose(pOutFile); + pOutFile = NULL; + + #ifdef _WIN32 + RetVal = _pclose(pPipe); + #else + RetVal = pclose(pPipe); + #endif + pPipe = NULL; + + if (RetVal != 0) { + DebugError (NULL, 0, 0003, "Error parsing file", "C preprocessor returned non-zero exit status for VFR file %s\n", mOptions.VfrFileName); + goto Fail; + } + + if (PreProcessCmd != NULL) { + delete[] PreProcessCmd; + } Out: SET_RUN_STATUS (STATUS_PREPROCESSED); return; Fail: + if (PreProcessCmd != NULL) { + delete[] PreProcessCmd; + } if (!IS_RUN_STATUS(STATUS_DEAD)) { SET_RUN_STATUS (STATUS_FAILED); } - delete[] PreProcessCmd; } extern UINT8 VfrParserStart (IN FILE *, IN INPUT_INFO_TO_SYNTAX *); diff --git a/BaseTools/Source/C/VolInfo/VolInfo.c b/BaseTools/Source/C/VolInfo/VolInfo.c index 428011ca04..d145182e33 100644 --- a/BaseTools/Source/C/VolInfo/VolInfo.c +++ b/BaseTools/Source/C/VolInfo/VolInfo.c @@ -39,6 +39,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include "ParseInf.h" #include "PeCoffLib.h" +#ifndef NATIVE_PATH_SEP +#ifdef _WIN32 +#define NATIVE_PATH_SEP '\\' +#else +#define NATIVE_PATH_SEP '/' +#endif +#endif + // // Utility global variables // @@ -141,7 +149,8 @@ EFI_STATUS CombinePath ( IN CHAR8* DefaultPath, IN CHAR8* AppendPath, - OUT CHAR8* NewPath + OUT CHAR8* NewPath, + IN size_t NewPathLen ); void @@ -335,12 +344,18 @@ Returns: // // We add quotes to the Openssl Path in case it has space characters // - OpenSslPath = malloc(2+strlen(OpenSslEnv)+strlen(OpenSslCommand)+1); + size_t need = strlen(OpenSslEnv) + strlen(OpenSslCommand) + 1 /*sep*/ + 2 /*quotes*/ + 1 /*NUL*/; + OpenSslPath = malloc(need); if (OpenSslPath == NULL) { Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!"); return GetUtilityStatus (); } - CombinePath(OpenSslEnv, OpenSslCommand, OpenSslPath); + if (CombinePath(OpenSslEnv, OpenSslCommand, OpenSslPath, need) != EFI_SUCCESS) { + free(OpenSslPath); + OpenSslPath = NULL; + Error (NULL, 0, 3001, "OpenSSL path combine failed", NULL); + return GetUtilityStatus(); + } } if (OpenSslPath == NULL){ Error (NULL, 0, 3000, "Open SSL command not available. Please verify PATH or set OPENSSL_PATH.", NULL); @@ -1665,29 +1680,21 @@ EFI_STATUS CombinePath ( IN CHAR8* DefaultPath, IN CHAR8* AppendPath, - OUT CHAR8* NewPath + OUT CHAR8* NewPath, + IN size_t NewPathLen ) { - UINT32 DefaultPathLen; - UINT64 Index; - CHAR8 QuotesStr[] = "\""; - strcpy(NewPath, QuotesStr); - DefaultPathLen = strlen(DefaultPath); - strcat(NewPath, DefaultPath); - Index = 0; - for (; Index < DefaultPathLen + 1; Index ++) { - if (NewPath[Index] == '\\' || NewPath[Index] == '/') { - if (NewPath[Index + 1] != '\0') { - NewPath[Index] = '/'; - } - } + if (NewPath == NULL || AppendPath == NULL) return EFI_INVALID_PARAMETER; + if (DefaultPath == NULL || DefaultPath[0] == '\0') { + if (snprintf(NewPath, NewPathLen, "%s", AppendPath) >= (int)NewPathLen) return EFI_ABORTED; + return EFI_SUCCESS; } - if (NewPath[Index -1] != '/') { - NewPath[Index] = '/'; - NewPath[Index + 1] = '\0'; + int needQuote = (strchr(DefaultPath, ' ') != NULL); + if (needQuote) { + if (snprintf(NewPath, NewPathLen, "\"%s%c%s\"", DefaultPath, NATIVE_PATH_SEP, AppendPath) >= (int)NewPathLen) return EFI_ABORTED; + } else { + if (snprintf(NewPath, NewPathLen, "%s%c%s", DefaultPath, NATIVE_PATH_SEP, AppendPath) >= (int)NewPathLen) return EFI_ABORTED; } - strcat(NewPath, AppendPath); - strcat(NewPath, QuotesStr); return EFI_SUCCESS; }