shithub: choc

Info  •  Files  •  Log  •  Branches

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.

--- a/textscreen/examples/guitest.c

+++ b/textscreen/examples/guitest.c

@@ -30,7 +30,10 @@

     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;

--- a/textscreen/txt_fileselect.c

+++ b/textscreen/txt_fileselect.c

@@ -333,31 +333,48 @@

 // 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 @@

     }

     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,22 +420,26 @@

     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;

+    const char *chooser;

+    char *ext_list = NULL;

+    char *window_title_escaped = NULL;

+    char *result = NULL;

+    unsigned int result_len = 64;

-    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.

@@ -414,8 +446,13 @@

     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 @@

     }

     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 *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 @@

     result = ExecReadOutput(argv);

     free(applescript);

-    free(window_title_copy);

     return result;

 }