Skip to content

Commit

Permalink
this patch eliminates a memory leak in the dynamic engine path genera…
Browse files Browse the repository at this point in the history
…tion under Windows-- the allocated memory for the path is now sufficient, and the additional code avoids the use of 'sprintf' family functions completely as Veracode would not stop complaining about passing environment variables into any of these functions (even the secure ones)
  • Loading branch information
flomar committed Sep 30, 2014
1 parent d9f8f7f commit f9e81fd
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 9 deletions.
52 changes: 47 additions & 5 deletions configuration.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,13 @@ int scep_conf_load(CONF *conf) {
if(v_flag)
printf("%s: Not setting a dynamic path. Not dynamic loading supported for engine %s\n", pname, scep_conf->engine->engine_id);
#ifdef WIN32
//fallback does not work yet!
//TODO: find out why it did not find the C:\Windows\System32\capi.dll
windir = getenv("WINDIR");
scep_conf->engine->dynamic_path = (char *) malloc(sizeof(SCEP_CONFIGURATION_DEFAULT_DYNAMICPATH_WINDOWS) + sizeof(scep_conf->engine->engine_id) + sizeof(windir));
sprintf(scep_conf->engine->dynamic_path, SCEP_CONFIGURATION_DEFAULT_DYNAMICPATH_WINDOWS, getenv("WINDIR"), scep_conf->engine->engine_id);
// this function creates the dynamic path to the engine based on
// the "%WINDIR%" environment variable, the system directory, and
// the engine identifier; in case it succeeds, the function writes
// the result into the last argument
if(!createEnginePath(getenv("WINDIR"), "System32", scep_conf->engine->engine_id, &scep_conf->engine->dynamic_path)) {
return 0;
}
#else
scep_conf->engine->dynamic_path = NULL;
#endif
Expand Down Expand Up @@ -509,3 +511,43 @@ void scep_dump_conf() {
}
exit(0);
}

#ifdef WIN32
// see header for details
int calculateArgumentLength(const char *_string, int *_length) {
int i;
for(i=0; i<CREATE_ENGINE_PATH_MAXIMUM_ARGUMENT_LENGTH; i++) {
if(_string[i] == 0) {
*_length = i;
return 1;
}
}
return 0;
}
// see header for details
int createEnginePath(const char *_directoryWindows, const char *_directorySystem, const char *_nameEngine, char **_result) {
int lengthDirectoryWindows = 0;
int lengthDirectorySystem = 0;
int lengthNameEngine = 0;
int lengthEnginePath = 0;
// try to calculate the argument lengths-- if any of the argument
// lengths exceed the supported argument length, we dump an error
// and return zero right away
if(!calculateArgumentLength(_directoryWindows, &lengthDirectoryWindows) || !calculateArgumentLength(_directorySystem, &lengthDirectorySystem) || !calculateArgumentLength(_nameEngine, &lengthNameEngine)) {
fprintf(stderr, "%s: one of the arguments (Windows directory, Windows system directory, or engine name) exceeds the maximum argument length\n", pname);
return 0;
}
// calculate total length of engine path, and allocate and initialize memory
lengthEnginePath = lengthDirectoryWindows + strlen("\\") + lengthDirectorySystem + strlen("\\") + lengthNameEngine + strlen(".dll");
*_result = (char*)(malloc(lengthEnginePath + 1));
memset(*_result, 0, lengthEnginePath + 1);
// construct engine path
memcpy(*_result + 0, _directoryWindows, lengthDirectoryWindows);
memcpy(*_result + lengthDirectoryWindows, "\\", strlen("\\"));
memcpy(*_result + lengthDirectoryWindows + strlen("\\"), _directorySystem, lengthDirectorySystem);
memcpy(*_result + lengthDirectoryWindows + strlen("\\") + lengthDirectorySystem, "\\", strlen("\\"));
memcpy(*_result + lengthDirectoryWindows + strlen("\\") + lengthDirectorySystem + strlen("\\"), _nameEngine, lengthNameEngine);
memcpy(*_result + lengthDirectoryWindows + strlen("\\") + lengthDirectorySystem + strlen("\\") + lengthNameEngine, ".dll", strlen(".dll"));
return 1;
}
#endif
25 changes: 21 additions & 4 deletions configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,6 @@
#define SCEP_CONFIGURATION_PARAM_MONITORINFO "MonitorInformation"
#define SCEP_CONFIGURATION_PARAM_SIGNERCERTIFICATE "SignerCertificateFile"

#ifdef WIN32
#define SCEP_CONFIGURATION_DEFAULT_DYNAMICPATH_WINDOWS "%s\\System32\\%s.dll"
#endif

/*
* Holds the configuration of all parts that are new,
* e.g. mostly engine stuff. In the futurue possibly
Expand Down Expand Up @@ -120,4 +116,25 @@ int scep_conf_load_operation_getnextca(CONF *conf);
void scep_dump_conf(void);
void error_memory(void);

#ifdef WIN32
// the maximum argument length supported by the "createEnginePath" function,
// this should be enough to cover even the longest file paths under Windows
#define CREATE_ENGINE_PATH_MAXIMUM_ARGUMENT_LENGTH 32768
// this function basically works like "strlen", but its behavior is not undefined
// if the first argument does not contain a terminating null character; basically
// this function just iterates over the first argument until it either hits a null
// character or exceeds the supported length; if successful, the resulting argument
// length is written to the last argument
int calculateArgumentLength(const char *_string, int *_length);
// this function is supposed to emulate what the old implementation did, but without
// passing environment variables (%WINDIR%) into the application unchecked; it takes
// three character pointers as arguments, the Windows directory (which should be
// injected using the %WINDIR% environment variable), the Windows system directory
// (something like "System32"), and the name of the engine (an arbitrary string);
// the result is written to the fourth argument, the required memory is implicity
// allocated
// EXAMPLE: "C:\Windows", "System32", "foobar" => "C:\Windows\System32\foobar.dll"
int createEnginePath(const char *_directoryWindows, const char *_directorySystem, const char *_nameEngine, char **_result);
#endif

#endif /* ifndef CONFIGURATION_H */

0 comments on commit f9e81fd

Please sign in to comment.