From bbf8c10e0db72dd602c84a7b1dcf4c3b5c886b46 Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Fri, 19 Jul 2019 15:44:24 -0700 Subject: [PATCH] ICU-20716 Fixing some buffer overruns in genccode --- icu4c/source/tools/genccode/genccode.c | 15 +- icu4c/source/tools/pkgdata/pkgdata.cpp | 25 ++- icu4c/source/tools/toolutil/pkg_genc.cpp | 242 +++++++++++++++++------ icu4c/source/tools/toolutil/pkg_genc.h | 25 ++- 4 files changed, 234 insertions(+), 73 deletions(-) diff --git a/icu4c/source/tools/genccode/genccode.c b/icu4c/source/tools/genccode/genccode.c index d35b5890105..91e94d7f518 100644 --- a/icu4c/source/tools/genccode/genccode.c +++ b/icu4c/source/tools/genccode/genccode.c @@ -63,6 +63,7 @@ enum { kOptHelpH = 0, kOptHelpQuestionMark, kOptDestDir, + kOptQuiet, kOptName, kOptEntryPoint, #ifdef CAN_GENERATE_OBJECTS @@ -77,6 +78,7 @@ static UOption options[]={ /*0*/UOPTION_HELP_H, UOPTION_HELP_QUESTION_MARK, UOPTION_DESTDIR, + UOPTION_QUIET, UOPTION_DEF("name", 'n', UOPT_REQUIRES_ARG), UOPTION_DEF("entrypoint", 'e', UOPT_REQUIRES_ARG), #ifdef CAN_GENERATE_OBJECTS @@ -116,6 +118,7 @@ main(int argc, char* argv[]) { "options:\n" "\t-h or -? or --help this usage text\n" "\t-d or --destdir destination directory, followed by the path\n" + "\t-q or --quiet do not display warnings and progress\n" "\t-n or --name symbol prefix, followed by the prefix\n" "\t-e or --entrypoint entry point name, followed by the name (_dat will be appended)\n" "\t-r or --revision Specify a version\n" @@ -159,6 +162,9 @@ main(int argc, char* argv[]) { writeCode = CALL_WRITECCODE; /* TODO: remove writeCode=&writeCCode; */ } + if (options[kOptQuiet].doesOccur) { + verbose = FALSE; + } while(--argc) { filename=getLongPathname(argv[argc]); if (verbose) { @@ -170,13 +176,15 @@ main(int argc, char* argv[]) { writeCCode(filename, options[kOptDestDir].value, options[kOptName].doesOccur ? options[kOptName].value : NULL, options[kOptFilename].doesOccur ? options[kOptFilename].value : NULL, - NULL); + NULL, + 0); break; case CALL_WRITEASSEMBLY: writeAssemblyCode(filename, options[kOptDestDir].value, options[kOptEntryPoint].doesOccur ? options[kOptEntryPoint].value : NULL, options[kOptFilename].doesOccur ? options[kOptFilename].value : NULL, - NULL); + NULL, + 0); break; #ifdef CAN_GENERATE_OBJECTS case CALL_WRITEOBJECT: @@ -184,7 +192,8 @@ main(int argc, char* argv[]) { options[kOptEntryPoint].doesOccur ? options[kOptEntryPoint].value : NULL, options[kOptMatchArch].doesOccur ? options[kOptMatchArch].value : NULL, options[kOptFilename].doesOccur ? options[kOptFilename].value : NULL, - NULL); + NULL, + 0); break; #endif default: diff --git a/icu4c/source/tools/pkgdata/pkgdata.cpp b/icu4c/source/tools/pkgdata/pkgdata.cpp index 226e4b3f6b7..429e4b4749f 100644 --- a/icu4c/source/tools/pkgdata/pkgdata.cpp +++ b/icu4c/source/tools/pkgdata/pkgdata.cpp @@ -718,7 +718,13 @@ static int32_t pkg_executeOptions(UPKGOptions *o) { if (genccodeAssembly && (uprv_strlen(genccodeAssembly)>3) && checkAssemblyHeaderName(genccodeAssembly+3)) { - writeAssemblyCode(datFileNamePath, o->tmpDir, o->entryName, NULL, gencFilePath); + writeAssemblyCode( + datFileNamePath, + o->tmpDir, + o->entryName, + NULL, + gencFilePath, + sizeof(gencFilePath)); result = pkg_createWithAssemblyCode(targetDir, mode, gencFilePath); if (result != 0) { @@ -753,7 +759,14 @@ static int32_t pkg_executeOptions(UPKGOptions *o) { /* Try to detect the arch type, use NULL if unsuccessful */ char optMatchArch[10] = { 0 }; pkg_createOptMatchArch(optMatchArch); - writeObjectCode(datFileNamePath, o->tmpDir, o->entryName, (optMatchArch[0] == 0 ? NULL : optMatchArch), NULL, gencFilePath); + writeObjectCode( + datFileNamePath, + o->tmpDir, + o->entryName, + (optMatchArch[0] == 0 ? NULL : optMatchArch), + NULL, + gencFilePath, + sizeof(gencFilePath)); pkg_destroyOptMatchArch(optMatchArch); #if U_PLATFORM_IS_LINUX_BASED result = pkg_generateLibraryFile(targetDir, mode, gencFilePath); @@ -1685,7 +1698,13 @@ static int32_t pkg_createWithoutAssemblyCode(UPKGOptions *o, const char *targetD printf("# Generating %s \n", gencmnFile); } - writeCCode(file, o->tmpDir, dataName[0] != 0 ? dataName : o->shortName, newName[0] != 0 ? newName : NULL, gencmnFile); + writeCCode( + file, + o->tmpDir, + dataName[0] != 0 ? dataName : o->shortName, + newName[0] != 0 ? newName : NULL, + gencmnFile, + sizeof(gencmnFile)); #ifdef USE_SINGLE_CCODE_FILE sprintf(cmd, "#include \"%s\"\n", gencmnFile); diff --git a/icu4c/source/tools/toolutil/pkg_genc.cpp b/icu4c/source/tools/toolutil/pkg_genc.cpp index ae8b3ece973..3f71e00cb64 100644 --- a/icu4c/source/tools/toolutil/pkg_genc.cpp +++ b/icu4c/source/tools/toolutil/pkg_genc.cpp @@ -48,6 +48,8 @@ #include "uoptions.h" #include "pkg_genc.h" #include "filetools.h" +#include "charstr.h" +#include "unicode/errorcode.h" #define MAX_COLUMN ((uint32_t)(0xFFFFFFFFU)) @@ -56,7 +58,15 @@ /* prototypes --------------------------------------------------------------- */ static void -getOutFilename(const char *inFilename, const char *destdir, char *outFilename, char *entryName, const char *newSuffix, const char *optFilename); +getOutFilename( + const char *inFilename, + const char *destdir, + char *outFilename, + int32_t outFilenameCapacity, + char *entryName, + int32_t entryNameCapacity, + const char *newSuffix, + const char *optFilename); static uint32_t write8(FileStream *out, uint8_t byte, uint32_t column); @@ -259,13 +269,21 @@ printAssemblyHeadersToStdErr(void) { } U_CAPI void U_EXPORT2 -writeAssemblyCode(const char *filename, const char *destdir, const char *optEntryPoint, const char *optFilename, char *outFilePath) { +writeAssemblyCode( + const char *filename, + const char *destdir, + const char *optEntryPoint, + const char *optFilename, + char *outFilePath, + size_t outFilePathCapacity) { uint32_t column = MAX_COLUMN; - char entry[64]; - uint32_t buffer[1024]; - char *bufferStr = (char *)buffer; + char entry[96]; + union { + uint32_t uint32s[1024]; + char chars[4096]; + } buffer; FileStream *in, *out; - size_t i, length; + size_t i, length, count; in=T_FileStream_open(filename, "rb"); if(in==NULL) { @@ -273,15 +291,27 @@ writeAssemblyCode(const char *filename, const char *destdir, const char *optEntr exit(U_FILE_ACCESS_ERROR); } - getOutFilename(filename, destdir, bufferStr, entry, ".S", optFilename); - out=T_FileStream_open(bufferStr, "w"); + getOutFilename( + filename, + destdir, + buffer.chars, + sizeof(buffer.chars), + entry, + sizeof(entry), + ".S", + optFilename); + out=T_FileStream_open(buffer.chars, "w"); if(out==NULL) { - fprintf(stderr, "genccode: unable to open output file %s\n", bufferStr); + fprintf(stderr, "genccode: unable to open output file %s\n", buffer.chars); exit(U_FILE_ACCESS_ERROR); } if (outFilePath != NULL) { - uprv_strcpy(outFilePath, bufferStr); + if (uprv_strlen(buffer.chars) >= outFilePathCapacity) { + fprintf(stderr, "genccode: filename too long\n"); + exit(U_ILLEGAL_ARGUMENT_ERROR); + } + uprv_strcpy(outFilePath, buffer.chars); } #if defined (WINDOWS_WITH_GNUC) && U_PLATFORM != U_PF_CYGWIN @@ -302,29 +332,42 @@ writeAssemblyCode(const char *filename, const char *destdir, const char *optEntr } } - sprintf(bufferStr, assemblyHeader[assemblyHeaderIndex].header, + count = snprintf( + buffer.chars, sizeof(buffer.chars), + assemblyHeader[assemblyHeaderIndex].header, entry, entry, entry, entry, entry, entry, entry, entry); - T_FileStream_writeLine(out, bufferStr); + if (count >= sizeof(buffer.chars)) { + fprintf(stderr, "genccode: entry name too long (long filename?)\n"); + exit(U_ILLEGAL_ARGUMENT_ERROR); + } + T_FileStream_writeLine(out, buffer.chars); T_FileStream_writeLine(out, assemblyHeader[assemblyHeaderIndex].beginLine); for(;;) { - memset(buffer, 0, sizeof(buffer)); - length=T_FileStream_read(in, buffer, sizeof(buffer)); + memset(buffer.uint32s, 0, sizeof(buffer.uint32s)); + length=T_FileStream_read(in, buffer.uint32s, sizeof(buffer.uint32s)); if(length==0) { break; } - for(i=0; i<(length/sizeof(buffer[0])); i++) { - column = write32(out, buffer[i], column); + for(i=0; i<(length/sizeof(buffer.uint32s[0])); i++) { + // TODO: What if the last read sees length not as a multiple of 4? + column = write32(out, buffer.uint32s[i], column); } } T_FileStream_writeLine(out, "\n"); - sprintf(bufferStr, assemblyHeader[assemblyHeaderIndex].footer, + count = snprintf( + buffer.chars, sizeof(buffer.chars), + assemblyHeader[assemblyHeaderIndex].footer, entry, entry, entry, entry, entry, entry, entry, entry); - T_FileStream_writeLine(out, bufferStr); + if (count >= sizeof(buffer.chars)) { + fprintf(stderr, "genccode: entry name too long (long filename?)\n"); + exit(U_ILLEGAL_ARGUMENT_ERROR); + } + T_FileStream_writeLine(out, buffer.chars); if(T_FileStream_error(in)) { fprintf(stderr, "genccode: file read error while generating from file %s\n", filename); @@ -341,11 +384,17 @@ writeAssemblyCode(const char *filename, const char *destdir, const char *optEntr } U_CAPI void U_EXPORT2 -writeCCode(const char *filename, const char *destdir, const char *optName, const char *optFilename, char *outFilePath) { +writeCCode( + const char *filename, + const char *destdir, + const char *optName, + const char *optFilename, + char *outFilePath, + size_t outFilePathCapacity) { uint32_t column = MAX_COLUMN; - char buffer[4096], entry[64]; + char buffer[4096], entry[96]; FileStream *in, *out; - size_t i, length; + size_t i, length, count; in=T_FileStream_open(filename, "rb"); if(in==NULL) { @@ -354,16 +403,35 @@ writeCCode(const char *filename, const char *destdir, const char *optName, const } if(optName != NULL) { /* prepend 'icudt28_' */ - strcpy(entry, optName); - strcat(entry, "_"); + // +2 includes the _ and the NUL + if (uprv_strlen(optName) + 2 > sizeof(entry)) { + fprintf(stderr, "genccode: entry name too long (long filename?)\n"); + exit(U_ILLEGAL_ARGUMENT_ERROR); + } + strcpy(entry, optName); + strcat(entry, "_"); } else { - entry[0] = 0; + entry[0] = 0; } - getOutFilename(filename, destdir, buffer, entry+uprv_strlen(entry), ".c", optFilename); + getOutFilename( + filename, + destdir, + buffer, + sizeof(buffer), + entry + uprv_strlen(entry), + sizeof(entry) - uprv_strlen(entry), + ".c", + optFilename); + if (outFilePath != NULL) { + if (uprv_strlen(buffer) >= outFilePathCapacity) { + fprintf(stderr, "genccode: filename too long\n"); + exit(U_ILLEGAL_ARGUMENT_ERROR); + } uprv_strcpy(outFilePath, buffer); } + out=T_FileStream_open(buffer, "w"); if(out==NULL) { fprintf(stderr, "genccode: unable to open output file %s\n", buffer); @@ -391,7 +459,7 @@ writeCCode(const char *filename, const char *destdir, const char *optName, const magic numbers we must still use the initial double. [grhoten 4/24/2003] */ - sprintf(buffer, + count = snprintf(buffer, sizeof(buffer), "#ifndef IN_GENERATED_CCODE\n" "#define IN_GENERATED_CCODE\n" "#define U_DISABLE_RENAMING 1\n" @@ -403,6 +471,10 @@ writeCCode(const char *filename, const char *destdir, const char *optName, const " const char *bytes; \n" "} %s={ 0.0, \n", entry); + if (count >= sizeof(buffer)) { + fprintf(stderr, "genccode: entry name too long (long filename?)\n"); + exit(U_ILLEGAL_ARGUMENT_ERROR); + } T_FileStream_writeLine(out, buffer); for(;;) { @@ -418,7 +490,7 @@ writeCCode(const char *filename, const char *destdir, const char *optName, const T_FileStream_writeLine(out, "\"\n};\nU_CDECL_END\n"); #else /* Function renaming shouldn't be done in data */ - sprintf(buffer, + count = snprintf(buffer, sizeof(buffer), "#ifndef IN_GENERATED_CCODE\n" "#define IN_GENERATED_CCODE\n" "#define U_DISABLE_RENAMING 1\n" @@ -430,6 +502,10 @@ writeCCode(const char *filename, const char *destdir, const char *optName, const " uint8_t bytes[%ld]; \n" "} %s={ 0.0, {\n", (long)T_FileStream_size(in), entry); + if (count >= sizeof(buffer)) { + fprintf(stderr, "genccode: entry name too long (long filename?)\n"); + exit(U_ILLEGAL_ARGUMENT_ERROR); + } T_FileStream_writeLine(out, buffer); for(;;) { @@ -583,66 +659,84 @@ write8str(FileStream *out, uint8_t byte, uint32_t column) { #endif static void -getOutFilename(const char *inFilename, const char *destdir, char *outFilename, char *entryName, const char *newSuffix, const char *optFilename) { +getOutFilename( + const char *inFilename, + const char *destdir, + char *outFilename, + int32_t outFilenameCapacity, + char *entryName, + int32_t entryNameCapacity, + const char *newSuffix, + const char *optFilename) { const char *basename=findBasename(inFilename), *suffix=uprv_strrchr(basename, '.'); + icu::CharString outFilenameBuilder; + icu::CharString entryNameBuilder; + icu::ErrorCode status; + /* copy path */ if(destdir!=NULL && *destdir!=0) { - do { - *outFilename++=*destdir++; - } while(*destdir!=0); - if(*(outFilename-1)!=U_FILE_SEP_CHAR) { - *outFilename++=U_FILE_SEP_CHAR; - } - inFilename=basename; + outFilenameBuilder.append(destdir, status); + outFilenameBuilder.ensureEndsWithFileSeparator(status); } else { - while(inFilename= outFilenameCapacity) { + fprintf(stderr, "genccode: output filename too long\n"); + exit(U_ILLEGAL_ARGUMENT_ERROR); + } + + if (entryNameBuilder.length() >= entryNameCapacity) { + fprintf(stderr, "genccode: entry name too long (long filename?)\n"); + exit(U_ILLEGAL_ARGUMENT_ERROR); + } + + uprv_strcpy(outFilename, outFilenameBuilder.data()); + uprv_strcpy(entryName, entryNameBuilder.data()); } #ifdef CAN_GENERATE_OBJECTS @@ -777,7 +871,14 @@ getArchitecture(uint16_t *pCPU, uint16_t *pBits, UBool *pIsBigEndian, const char } U_CAPI void U_EXPORT2 -writeObjectCode(const char *filename, const char *destdir, const char *optEntryPoint, const char *optMatchArch, const char *optFilename, char *outFilePath) { +writeObjectCode( + const char *filename, + const char *destdir, + const char *optEntryPoint, + const char *optMatchArch, + const char *optFilename, + char *outFilePath, + size_t outFilePathCapacity) { /* common variables */ char buffer[4096], entry[96]={ 0 }; FileStream *in, *out; @@ -1061,8 +1162,21 @@ writeObjectCode(const char *filename, const char *destdir, const char *optEntryP } size=T_FileStream_size(in); - getOutFilename(filename, destdir, buffer, entry+entryOffset, newSuffix, optFilename); + getOutFilename( + filename, + destdir, + buffer, + sizeof(buffer), + entry + entryOffset, + sizeof(entry) - entryOffset, + newSuffix, + optFilename); + if (outFilePath != NULL) { + if (uprv_strlen(buffer) >= outFilePathCapacity) { + fprintf(stderr, "genccode: filename too long\n"); + exit(U_ILLEGAL_ARGUMENT_ERROR); + } uprv_strcpy(outFilePath, buffer); } diff --git a/icu4c/source/tools/toolutil/pkg_genc.h b/icu4c/source/tools/toolutil/pkg_genc.h index 5039f27db5e..47e8304a689 100644 --- a/icu4c/source/tools/toolutil/pkg_genc.h +++ b/icu4c/source/tools/toolutil/pkg_genc.h @@ -75,12 +75,31 @@ U_INTERNAL UBool U_EXPORT2 checkAssemblyHeaderName(const char* optAssembly); U_INTERNAL void U_EXPORT2 -writeCCode(const char *filename, const char *destdir, const char *optName, const char *optFilename, char *outFilePath); +writeCCode( + const char *filename, + const char *destdir, + const char *optName, + const char *optFilename, + char *outFilePath, + size_t outFilePathCapacity); U_INTERNAL void U_EXPORT2 -writeAssemblyCode(const char *filename, const char *destdir, const char *optEntryPoint, const char *optFilename, char *outFilePath); +writeAssemblyCode( + const char *filename, + const char *destdir, + const char *optEntryPoint, + const char *optFilename, + char *outFilePath, + size_t outFilePathCapacity); U_INTERNAL void U_EXPORT2 -writeObjectCode(const char *filename, const char *destdir, const char *optEntryPoint, const char *optMatchArch, const char *optFilename, char *outFilePath); +writeObjectCode( + const char *filename, + const char *destdir, + const char *optEntryPoint, + const char *optMatchArch, + const char *optFilename, + char *outFilePath, + size_t outFilePathCapacity); #endif