commit cc8faa989c678a7b76688def831f5fed996bcd8c
Author: Ioan Chera <ioan.chera@gmail.com>
AuthorDate: Sun Mar 18 12:08:35 2018 +0200
Commit: Ioan Chera <ioan.chera@gmail.com>
CommitDate: Sun Mar 18 12:08:35 2018 +0200
Clean-up of AppleScript-generating C string processing
* Functions which return something that must be freed now have a Create or Generate in their name
* Fix a buffer overflow risk in CreateEscapedString
* Ensure const-correctness
* Verify malloc result
* Updated guitest with file extensions which would have crashed the previous code.
---
textscreen/examples/guitest.c | 5 +-
textscreen/txt_fileselect.c | 112 ++++++++++++++++++++++++++++++------------
2 files changed, 84 insertions(+), 33 deletions(-)
diff --git a/textscreen/examples/guitest.c b/textscreen/examples/guitest.c
index 5c736055..2330d61d 100644
--- a/textscreen/examples/guitest.c
+++ b/textscreen/examples/guitest.c
@@ -30,7 +30,10 @@ enum
RADIO_VALUE_MUSHROOM,
RADIO_VALUE_SNAKE,
};
-char *extensions[] = { "wad", "lmp", "txt", NULL };
+
+// also put some crazy extensions to test the escape function. a"b"c"""dd
+char *extensions[] = { "wad", "lmp", "txt", "a\"b\"c\"\"\"dd", "",
+ "\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"", NULL };
char *radio_values[] = { "Badger", "Mushroom", "Snake" };
char *textbox_value = NULL;
int numbox_value = 0;
diff --git a/textscreen/txt_fileselect.c b/textscreen/txt_fileselect.c
index 89e51aa5..7bcbac29 100644
--- a/textscreen/txt_fileselect.c
+++ b/textscreen/txt_fileselect.c
@@ -333,31 +333,48 @@ char *TXT_SelectFile(char *window_title, char **extensions)
// Printf format string for the "wrapper" portion of the AppleScript:
#define APPLESCRIPT_WRAPPER "copy POSIX path of (%s) to stdout"
-static char *EscapedString(char *s)
+static char *CreateEscapedString(const char *original)
{
char *result;
- char *in, *out;
+ const char *in;
+ char *out;
- result = malloc(strlen(s) + 3);
+ // We need to take care not to overflow the buffer, so count exactly.
+#define ESCAPED_CHARS "\"\\"
+ size_t count_extras = 2; // start counting the two quotes
+ for (in = original; *in; ++in)
+ {
+ if (strchr(ESCAPED_CHARS, *in))
+ {
+ ++count_extras;
+ }
+ }
+
+ result = malloc(strlen(original) + count_extras + 1);
+ if (!result)
+ {
+ return NULL;
+ }
out = result;
- *out++ = '\"';
- for (in = s; *in != '\0'; ++in)
+ *out++ = '"';
+ for (in = original; *in; ++in)
{
- if (*in == '\"' || *in == '\\')
+ if (strchr(ESCAPED_CHARS, *in))
{
*out++ = '\\';
}
*out++ = *in;
}
- *out++ = '\"';
- *out = '\0';
+ *out++ = '"';
+ *out = 0;
return result;
+#undef ESCAPED_CHARS
}
// Build list of extensions, like: {"wad","lmp","txt"}
-static char *ExtensionsList(char **extensions)
+static char *CreateExtensionsList(char **extensions)
{
char *result, *escaped;
unsigned int result_len;
@@ -375,16 +392,27 @@ static char *ExtensionsList(char **extensions)
}
result = malloc(result_len);
+ if (!result)
+ {
+ return NULL;
+ }
TXT_StringCopy(result, "{", result_len);
for (i = 0; extensions[i] != NULL; ++i)
{
- escaped = EscapedString(extensions[i]);
+ escaped = CreateEscapedString(extensions[i]);
+ if (!escaped)
+ {
+ free(result);
+ return NULL;
+ }
TXT_StringConcat(result, escaped, result_len);
free(escaped);
if (extensions[i + 1] != NULL)
+ {
TXT_StringConcat(result, ",", result_len);
+ }
}
TXT_StringConcat(result, "}", result_len);
@@ -392,30 +420,39 @@ static char *ExtensionsList(char **extensions)
return result;
}
-static char *GenerateSelector(char *window_title, char **extensions)
+static char *GenerateSelector(const char *const window_title, char **extensions)
{
- char *chooser, *ext_list, *result;
- unsigned int result_len;
-
- result_len = 64;
+ const char *chooser;
+ char *ext_list = NULL;
+ char *window_title_escaped = NULL;
+ char *result = NULL;
+ unsigned int result_len = 64;
if (extensions == TXT_DIRECTORY)
{
chooser = "choose folder";
- ext_list = NULL;
}
else
{
chooser = "choose file";
- ext_list = ExtensionsList(extensions);
+ ext_list = CreateExtensionsList(extensions);
+ if (!ext_list)
+ {
+ return NULL;
+ }
}
// Calculate size.
if (window_title != NULL)
{
- window_title = EscapedString(window_title);
- result_len += strlen(window_title);
+ window_title_escaped = CreateEscapedString(window_title);
+ if (!window_title_escaped)
+ {
+ free(ext_list);
+ return NULL;
+ }
+ result_len += strlen(window_title_escaped);
}
if (ext_list != NULL)
{
@@ -423,35 +460,51 @@ static char *GenerateSelector(char *window_title, char **extensions)
}
result = malloc(result_len);
+ if (!result)
+ {
+ free(window_title_escaped);
+ free(ext_list);
+ return NULL;
+ }
TXT_StringCopy(result, chooser, result_len);
- if (window_title != NULL)
+ if (window_title_escaped != NULL)
{
TXT_StringConcat(result, " with prompt ", result_len);
- TXT_StringConcat(result, window_title, result_len);
- free(window_title);
+ TXT_StringConcat(result, window_title_escaped, result_len);
}
if (ext_list != NULL)
{
- TXT_StringConcat(result, "of type ", result_len);
+ TXT_StringConcat(result, " of type ", result_len);
TXT_StringConcat(result, ext_list, result_len);
- free(ext_list);
}
+ free(window_title_escaped);
+ free(ext_list);
return result;
}
-static char *GenerateAppleScript(char *window_title, char **extensions)
+static char *GenerateAppleScript(const char *window_title, char **extensions)
{
char *selector, *result;
size_t result_len;
selector = GenerateSelector(window_title, extensions);
+ if (!selector)
+ {
+ return NULL;
+ }
result_len = strlen(APPLESCRIPT_WRAPPER) + strlen(selector);
result = malloc(result_len);
+ if (!result)
+ {
+ free(selector);
+ return NULL;
+ }
+
TXT_snprintf(result, result_len, APPLESCRIPT_WRAPPER, selector);
free(selector);
@@ -468,16 +521,12 @@ char *TXT_SelectFile(const char *window_title, char **extensions)
char *argv[4];
char *result, *applescript;
- // We need to create a temporary copy of window_title because GenerateApple-
- // Script works with non-const string.
- char *window_title_copy = strdup(window_title);
- if (!window_title_copy)
+ applescript = GenerateAppleScript(window_title, extensions);
+ if (!applescript)
{
return NULL;
}
- applescript = GenerateAppleScript(window_title_copy, extensions);
-
argv[0] = "/usr/bin/osascript";
argv[1] = "-e";
argv[2] = applescript;
@@ -486,7 +535,6 @@ char *TXT_SelectFile(const char *window_title, char **extensions)
result = ExecReadOutput(argv);
free(applescript);
- free(window_title_copy);
return result;
}