[Asterisk-code-review] curl, res_stir_shaken: refactor utility functions (asterisk[master])
N A
asteriskteam at digium.com
Thu Dec 16 13:43:59 CST 2021
N A has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/17708 )
Change subject: curl, res_stir_shaken: refactor utility functions
......................................................................
curl, res_stir_shaken: refactor utility functions
Refactors curl and temp file utility functions
into file.c and utils.c.
ASTERISK-29809 #close
Change-Id: Ife478708c8f2b127239cb73c1755ef18c0bf431b
---
M funcs/func_curl.c
M include/asterisk/file.h
M include/asterisk/utils.h
M main/file.c
M main/utils.c
M res/res_stir_shaken/curl.c
6 files changed, 66 insertions(+), 62 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/08/17708/1
diff --git a/funcs/func_curl.c b/funcs/func_curl.c
index 3b04f12..9b15b2d 100644
--- a/funcs/func_curl.c
+++ b/funcs/func_curl.c
@@ -629,31 +629,6 @@
AST_THREADSTORAGE_CUSTOM(curl_instance, curl_instance_init, curl_instance_cleanup);
AST_THREADSTORAGE(thread_escapebuf);
-/*!
- * \brief Check for potential HTTP injection risk.
- *
- * CVE-2014-8150 brought up the fact that HTTP proxies are subject to injection
- * attacks. An HTTP URL sent to a proxy contains a carriage-return linefeed combination,
- * followed by a complete HTTP request. Proxies will handle this as two separate HTTP
- * requests rather than as a malformed URL.
- *
- * libcURL patched this vulnerability in version 7.40.0, but we have no guarantee that
- * Asterisk systems will be using an up-to-date cURL library. Therefore, we implement
- * the same fix as libcURL for determining if a URL is vulnerable to an injection attack.
- *
- * \param url The URL to check for vulnerability
- * \retval 0 The URL is not vulnerable
- * \retval 1 The URL is vulnerable.
- */
-static int url_is_vulnerable(const char *url)
-{
- if (strpbrk(url, "\r\n")) {
- return 1;
- }
-
- return 0;
-}
-
struct curl_args {
const char *url;
const char *postdata;
@@ -685,7 +660,7 @@
return -1;
}
- if (url_is_vulnerable(args->url)) {
+ if (ast_url_is_vulnerable(args->url)) {
ast_log(LOG_ERROR, "URL '%s' is vulnerable to HTTP injection attacks. Aborting CURL() call.\n", args->url);
return -1;
}
@@ -961,14 +936,14 @@
}
for (i = 0; i < ARRAY_LEN(bad_urls); ++i) {
- if (!url_is_vulnerable(bad_urls[i])) {
+ if (!ast_url_is_vulnerable(bad_urls[i])) {
ast_test_status_update(test, "String '%s' detected as valid when it should be invalid\n", bad_urls[i]);
res = AST_TEST_FAIL;
}
}
for (i = 0; i < ARRAY_LEN(good_urls); ++i) {
- if (url_is_vulnerable(good_urls[i])) {
+ if (ast_url_is_vulnerable(good_urls[i])) {
ast_test_status_update(test, "String '%s' detected as invalid when it should be valid\n", good_urls[i]);
res = AST_TEST_FAIL;
}
diff --git a/include/asterisk/file.h b/include/asterisk/file.h
index b2da2a2..92b65e7 100644
--- a/include/asterisk/file.h
+++ b/include/asterisk/file.h
@@ -147,6 +147,19 @@
FILE *ast_file_mkftemp(char *template, mode_t mode);
/*!
+ * \brief Create a temporary file located at path
+ *
+ * \note This function assumes path does not end with a '/'
+ *
+ * \param path The directory path to create the file in
+ * \param filename Function allocates memory and stores full filename (including path) here
+ *
+ * \retval -1 on failure
+ * \return file descriptor on success
+ */
+int ast_create_temp_file(const char *path, char **filename);
+
+/*!
* \brief Callback called for each file found when reading directories
* \param dir_name the name of the directory
* \param filename the name of the file
diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h
index 72deaa3..07c5cbb 100644
--- a/include/asterisk/utils.h
+++ b/include/asterisk/utils.h
@@ -396,6 +396,24 @@
*/
void ast_uri_decode(char *s, struct ast_flags spec);
+/*!
+ * \brief Check for potential HTTP injection risk.
+ *
+ * CVE-2014-8150 brought up the fact that HTTP proxies are subject to injection
+ * attacks. An HTTP URL sent to a proxy contains a carriage-return linefeed combination,
+ * followed by a complete HTTP request. Proxies will handle this as two separate HTTP
+ * requests rather than as a malformed URL.
+ *
+ * libcURL patched this vulnerability in version 7.40.0, but we have no guarantee that
+ * Asterisk systems will be using an up-to-date cURL library. Therefore, we implement
+ * the same fix as libcURL for determining if a URL is vulnerable to an injection attack.
+ *
+ * \param url The URL to check for vulnerability
+ * \retval 0 The URL is not vulnerable
+ * \retval 1 The URL is vulnerable.
+ */
+int ast_url_is_vulnerable(const char *url);
+
/*! ast_xml_escape
\brief Escape reserved characters for use in XML.
diff --git a/main/file.c b/main/file.c
index 0c1fdd4..d5afd05 100644
--- a/main/file.c
+++ b/main/file.c
@@ -199,6 +199,26 @@
return p;
}
+int create_temp_file(const char *path, char **filename, const char *template_name)
+{
+ int fd;
+
+ if (ast_asprintf(filename, "%s/%s", path, template_name) < 0) {
+ ast_log(LOG_ERROR, "Failed to set up temporary file path\n");
+ return -1;
+ }
+
+ ast_mkdir(path, 0644);
+
+ if ((fd = mkstemp(*filename)) < 0) {
+ ast_log(LOG_NOTICE, "Failed to create temporary file\n");
+ ast_free(*filename);
+ return -1;
+ }
+
+ return fd;
+}
+
int ast_stopstream(struct ast_channel *tmp)
{
ast_channel_lock(tmp);
diff --git a/main/utils.c b/main/utils.c
index dc94c99..cf10167 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -776,6 +776,15 @@
*o = '\0';
}
+int ast_url_is_vulnerable(const char *url)
+{
+ if (strpbrk(url, "\r\n")) {
+ return 1;
+ }
+
+ return 0;
+}
+
char *ast_escape_quoted(const char *string, char *outbuf, int buflen)
{
const char *ptr = string;
diff --git a/res/res_stir_shaken/curl.c b/res/res_stir_shaken/curl.c
index ad3adbc..f98608b 100644
--- a/res/res_stir_shaken/curl.c
+++ b/res/res_stir_shaken/curl.c
@@ -151,42 +151,11 @@
return curl;
}
-/*!
- * \brief Create a temporary file located at path
- *
- * \note This function assumes path does not end with a '/'
- *
- * \param path The directory path to create the file in
- * \param filename Function allocates memory and stores full filename (including path) here
- *
- * \retval -1 on failure
- * \return file descriptor on success
- */
-static int create_temp_file(const char *path, char **filename)
-{
- const char *template_name = "certXXXXXX";
- int fd;
-
- if (ast_asprintf(filename, "%s/%s", path, template_name) < 0) {
- ast_log(LOG_ERROR, "Failed to set up temporary file path for CURL\n");
- return -1;
- }
-
- ast_mkdir(path, 0644);
-
- if ((fd = mkstemp(*filename)) < 0) {
- ast_log(LOG_NOTICE, "Failed to create temporary file for CURL\n");
- ast_free(*filename);
- return -1;
- }
-
- return fd;
-}
-
char *curl_public_key(const char *public_cert_url, const char *path, struct curl_cb_data *data)
{
FILE *public_key_file;
RAII_VAR(char *, tmp_filename, NULL, ast_free);
+ const char *template_name = "certXXXXXX";
char *filename;
char *serial;
int fd;
@@ -199,9 +168,9 @@
/* For now, it's fine to pass in path as is - it shouldn't end with a '/'. However,
* if we decide to change how certificates are stored in the future (configurable paths),
* then we will need to check to see if path ends with '/', copy everything up to the '/',
- * and use this new variable for create_temp_file as well as for ast_asprintf below.
+ * and use this new variable for ast_create_temp_file as well as for ast_asprintf below.
*/
- fd = create_temp_file(path, &tmp_filename);
+ fd = ast_create_temp_file(path, &tmp_filename, template_name);
if (fd == -1) {
ast_log(LOG_ERROR, "Failed to get temporary file descriptor for CURL\n");
return NULL;
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/17708
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Ife478708c8f2b127239cb73c1755ef18c0bf431b
Gerrit-Change-Number: 17708
Gerrit-PatchSet: 1
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20211216/82de3e50/attachment.html>
More information about the asterisk-code-review
mailing list