ICU-20716 Fixing some buffer overruns in genccode
diff --git a/icu4c/source/tools/genccode/genccode.c b/icu4c/source/tools/genccode/genccode.c
index d35b589..91e94d7 100644
--- a/icu4c/source/tools/genccode/genccode.c
+++ b/icu4c/source/tools/genccode/genccode.c
@@ -63,6 +63,7 @@
kOptHelpH = 0,
kOptHelpQuestionMark,
kOptDestDir,
+ kOptQuiet,
kOptName,
kOptEntryPoint,
#ifdef CAN_GENERATE_OBJECTS
@@ -77,6 +78,7 @@
/*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 @@
"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 @@
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 @@
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 @@
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 226e4b3..429e4b4 100644
--- a/icu4c/source/tools/pkgdata/pkgdata.cpp
+++ b/icu4c/source/tools/pkgdata/pkgdata.cpp
@@ -718,7 +718,13 @@
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 @@
/* 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 @@
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 ae8b3ec..3f71e00 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 @@
}
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 @@
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 @@
}
}
- 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 @@
}
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 @@
}
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 @@
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 @@
" 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 @@
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 @@
" 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 @@
#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<basename) {
- *outFilename++=*inFilename++;
- }
+ outFilenameBuilder.append(inFilename, basename - inFilename, status);
}
+ inFilename=basename;
if(suffix==NULL) {
/* the filename does not have a suffix */
- uprv_strcpy(entryName, inFilename);
+ entryNameBuilder.append(inFilename, status);
if(optFilename != NULL) {
- uprv_strcpy(outFilename, optFilename);
+ outFilenameBuilder.append(optFilename, status);
} else {
- uprv_strcpy(outFilename, inFilename);
+ outFilenameBuilder.append(inFilename, status);
}
- uprv_strcat(outFilename, newSuffix);
+ outFilenameBuilder.append(newSuffix, status);
} else {
- char *saveOutFilename = outFilename;
+ int32_t saveOutFilenameLength = outFilenameBuilder.length();
/* copy basename */
while(inFilename<suffix) {
- if(*inFilename=='-') {
- /* iSeries cannot have '-' in the .o objects. */
- *outFilename++=*entryName++='_';
- inFilename++;
- }
- else {
- *outFilename++=*entryName++=*inFilename++;
- }
+ // iSeries cannot have '-' in the .o objects.
+ char c = (*inFilename=='-') ? '_' : *inFilename;
+ outFilenameBuilder.append(c, status);
+ entryNameBuilder.append(c, status);
+ inFilename++;
}
/* replace '.' by '_' */
- *outFilename++=*entryName++='_';
+ outFilenameBuilder.append('_', status);
+ entryNameBuilder.append('_', status);
++inFilename;
/* copy suffix */
- while(*inFilename!=0) {
- *outFilename++=*entryName++=*inFilename++;
- }
-
- *entryName=0;
+ outFilenameBuilder.append(inFilename, status);
+ entryNameBuilder.append(inFilename, status);
if(optFilename != NULL) {
- uprv_strcpy(saveOutFilename, optFilename);
- uprv_strcat(saveOutFilename, newSuffix);
- } else {
- /* add ".c" */
- uprv_strcpy(outFilename, newSuffix);
+ outFilenameBuilder.truncate(saveOutFilenameLength);
+ outFilenameBuilder.append(optFilename, status);
}
+ // add ".c"
+ outFilenameBuilder.append(newSuffix, status);
}
+
+ if (status.isFailure()) {
+ fprintf(stderr, "genccode: error building filename or entrypoint\n");
+ exit(status.get());
+ }
+
+ if (outFilenameBuilder.length() >= 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 @@
}
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 @@
}
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 5039f27..47e8304 100644
--- a/icu4c/source/tools/toolutil/pkg_genc.h
+++ b/icu4c/source/tools/toolutil/pkg_genc.h
@@ -75,12 +75,31 @@
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
