From 9a7c28cd5aeb577740ccb37e9e740a177223a762 Mon Sep 17 00:00:00 2001
From: "Kevin P. Fleming" <kpfleming@digium.com>
Date: Sat, 29 Nov 2008 15:29:33 +0000
Subject: [PATCH] we can now build with -Wformat=2, which found a couple of
real bugs
because SPRINTF() use non-literal format strings (which cannot be checked), move it into its own module so the rest of func_strings can benefit from format string checking
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@159774 65c4cc65-6c06-0410-ace0-fbb531ad65f3
---
Makefile | 2 +-
UPGRADE.txt | 7 +-
channels/misdn/ie.c | 4 +-
channels/misdn_config.c | 14 +--
funcs/Makefile | 7 ++
funcs/func_sprintf.c | 230 ++++++++++++++++++++++++++++++++++++++++
funcs/func_strings.c | 170 -----------------------------
main/Makefile | 2 +-
res/res_config_sqlite.c | 13 ++-
9 files changed, 261 insertions(+), 188 deletions(-)
create mode 100644 funcs/func_sprintf.c
diff --git a/Makefile b/Makefile
index 45b01b7690..39b17c5f34 100644
--- a/Makefile
+++ b/Makefile
@@ -237,7 +237,7 @@ endif
ASTCFLAGS+=-Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations $(DEBUG)
ifeq ($(AST_DEVMODE),yes)
- ASTCFLAGS+=-Werror -Wunused -Wundef $(AST_DECLARATION_AFTER_STATEMENT) -Wmissing-format-attribute -Wformat-security #-Wformat=2
+ ASTCFLAGS+=-Werror -Wunused -Wundef $(AST_DECLARATION_AFTER_STATEMENT) -Wmissing-format-attribute -Wformat=2
endif
ifneq ($(findstring BSD,$(OSARCH)),)
diff --git a/UPGRADE.txt b/UPGRADE.txt
index 199da5ed51..ccfe1c5106 100644
--- a/UPGRADE.txt
+++ b/UPGRADE.txt
@@ -85,7 +85,7 @@ From 1.6.1 to 1.6.2:
has been replaced with 'UNSUPPORTED'). This change makes the
SendImage application more consistent with other applications.
-* skinny.conf now has seperate sections for lines and devices.
+* skinny.conf now has separate sections for lines and devices.
Please have a look at configs/skinny.conf.sample and update
your skinny.conf.
@@ -95,3 +95,8 @@ From 1.6.1 to 1.6.2:
case-insensitive comparisons now when originally hashing based on
queue names, meaning that now the two queues mentioned as examples
earlier will be seen as having the same name.
+
+* The SPRINTF() dialplan function has been moved into its own module,
+ func_sprintf, and is no longer included in func_strings. If you use this
+ function and do not use 'autoload=yes' in modules.conf, you will need
+ to explicitly load func_sprintf for it to be available.
diff --git a/channels/misdn/ie.c b/channels/misdn/ie.c
index fbd06d1636..dc9802845b 100644
--- a/channels/misdn/ie.c
+++ b/channels/misdn/ie.c
@@ -1347,7 +1347,7 @@ static void enc_ie_useruser(unsigned char **ntmode, msg_t *msg, int protocol, ch
i = 0;
while(i < user_len)
{
- if (MISDN_IE_DEBG) printf(debug+(i*3), " %02x", user[i]);
+ if (MISDN_IE_DEBG) sprintf(debug+(i*3), " %02x", user[i]);
i++;
}
@@ -1393,7 +1393,7 @@ static void dec_ie_useruser(unsigned char *p, Q931_info_t *qi, int *protocol, ch
i = 0;
while(i < *user_len)
{
- if (MISDN_IE_DEBG) printf(debug+(i*3), " %02x", user[i]);
+ if (MISDN_IE_DEBG) sprintf(debug+(i*3), " %02x", user[i]);
i++;
}
debug[i*3] = '\0';
diff --git a/channels/misdn_config.c b/channels/misdn_config.c
index 12a742cf34..29723e17c9 100644
--- a/channels/misdn_config.c
+++ b/channels/misdn_config.c
@@ -882,12 +882,14 @@ static int _parse (union misdn_cfg_pt *dest, const char *value, enum misdn_cfg_t
break;
case MISDN_CTYPE_INT:
{
- char *pat;
- if (strchr(value,'x'))
- pat="%x";
- else
- pat="%d";
- if (sscanf(value, pat, &tmp)) {
+ int res;
+
+ if (strchr(value,'x')) {
+ res = sscanf(value, "%x", &tmp);
+ } else {
+ res = sscanf(value, "%d", &tmp);
+ }
+ if (res) {
dest->num = ast_malloc(sizeof(int));
memcpy(dest->num, &tmp, sizeof(int));
} else
diff --git a/funcs/Makefile b/funcs/Makefile
index 9b13b17a0b..bc4745d081 100644
--- a/funcs/Makefile
+++ b/funcs/Makefile
@@ -18,3 +18,10 @@ MENUSELECT_DESCRIPTION=Dialplan Functions
all: _all
include $(ASTTOPDIR)/Makefile.moddir_rules
+
+# the SPRINTF() function in func_sprintf accepts format specifiers
+# and thus passes them to snprintf() as non-literal strings; the compiler
+# can't check the string and arguments to ensure they match, so this
+# warning must be disabled; for safety reasons, SPRINTF() is kept in
+# a separate module so that as little code as possible is left unchecked
+func_sprintf.o: ASTCFLAGS+=-Wno-format-nonliteral
diff --git a/funcs/func_sprintf.c b/funcs/func_sprintf.c
new file mode 100644
index 0000000000..af292df1d5
--- /dev/null
+++ b/funcs/func_sprintf.c
@@ -0,0 +1,230 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2005-2006, Digium, Inc.
+ * Portions Copyright (C) 2005, Tilghman Lesher. All rights reserved.
+ * Portions Copyright (C) 2005, Anthony Minessale II
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*! \file
+ *
+ * \brief String manipulation dialplan functions
+ *
+ * \author Tilghman Lesher
+ * \author Anothony Minessale II
+ * \ingroup functions
+ */
+
+#include "asterisk.h"
+
+ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
+
+#include <ctype.h>
+
+#include "asterisk/module.h"
+#include "asterisk/channel.h"
+#include "asterisk/pbx.h"
+#include "asterisk/utils.h"
+#include "asterisk/app.h"
+
+AST_THREADSTORAGE(result_buf);
+
+/*** DOCUMENTATION
+ <function name="SPRINTF" language="en_US">
+ <synopsis>
+ Format a variable according to a format string.
+ </synopsis>
+ <syntax>
+ <parameter name="format" required="true" />
+ <parameter name="arg1" required="true" />
+ <parameter name="arg2" multiple="true" />
+ <parameter name="argN" />
+ </syntax>
+ <description>
+ <para>Parses the format string specified and returns a string matching
+ that format. Supports most options found in <emphasis>sprintf(3)</emphasis>.
+ Returns a shortened string if a format specifier is not recognized.</para>
+ </description>
+ <see-also>
+ <ref type="manpage">sprintf(3)</ref>
+ </see-also>
+ </function>
+ ***/
+static int acf_sprintf(struct ast_channel *chan, const char *cmd, char *data, char *buf, size_t len)
+{
+#define SPRINTF_FLAG 0
+#define SPRINTF_WIDTH 1
+#define SPRINTF_PRECISION 2
+#define SPRINTF_LENGTH 3
+#define SPRINTF_CONVERSION 4
+ int i, state = -1, argcount = 0;
+ char *formatstart = NULL, *bufptr = buf;
+ char formatbuf[256] = "";
+ int tmpi;
+ double tmpd;
+ AST_DECLARE_APP_ARGS(arg,
+ AST_APP_ARG(format);
+ AST_APP_ARG(var)[100];
+ );
+
+ AST_STANDARD_APP_ARGS(arg, data);
+
+ /* Scan the format, converting each argument into the requisite format type. */
+ for (i = 0; arg.format[i]; i++) {
+ switch (state) {
+ case SPRINTF_FLAG:
+ if (strchr("#0- +'I", arg.format[i]))
+ break;
+ state = SPRINTF_WIDTH;
+ case SPRINTF_WIDTH:
+ if (arg.format[i] >= '0' && arg.format[i] <= '9')
+ break;
+
+ /* Next character must be a period to go into a precision */
+ if (arg.format[i] == '.') {
+ state = SPRINTF_PRECISION;
+ } else {
+ state = SPRINTF_LENGTH;
+ i--;
+ }
+ break;
+ case SPRINTF_PRECISION:
+ if (arg.format[i] >= '0' && arg.format[i] <= '9')
+ break;
+ state = SPRINTF_LENGTH;
+ case SPRINTF_LENGTH:
+ if (strchr("hl", arg.format[i])) {
+ if (arg.format[i + 1] == arg.format[i])
+ i++;
+ state = SPRINTF_CONVERSION;
+ break;
+ } else if (strchr("Lqjzt", arg.format[i])) {
+ state = SPRINTF_CONVERSION;
+ break;
+ }
+ state = SPRINTF_CONVERSION;
+ case SPRINTF_CONVERSION:
+ if (strchr("diouxXc", arg.format[i])) {
+ /* Integer */
+
+ /* Isolate this format alone */
+ ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
+ formatbuf[&arg.format[i] - formatstart + 1] = '\0';
+
+ /* Convert the argument into the required type */
+ if (arg.var[argcount]) {
+ if (sscanf(arg.var[argcount++], "%d", &tmpi) != 1) {
+ ast_log(LOG_ERROR, "Argument '%s' is not an integer number for format '%s'\n", arg.var[argcount - 1], formatbuf);
+ goto sprintf_fail;
+ }
+ } else {
+ ast_log(LOG_ERROR, "SPRINTF() has more format specifiers than arguments!\n");
+ goto sprintf_fail;
+ }
+
+ /* Format the argument */
+ snprintf(bufptr, buf + len - bufptr, formatbuf, tmpi);
+
+ /* Update the position of the next parameter to print */
+ bufptr = strchr(buf, '\0');
+ } else if (strchr("eEfFgGaA", arg.format[i])) {
+ /* Double */
+
+ /* Isolate this format alone */
+ ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
+ formatbuf[&arg.format[i] - formatstart + 1] = '\0';
+
+ /* Convert the argument into the required type */
+ if (arg.var[argcount]) {
+ if (sscanf(arg.var[argcount++], "%lf", &tmpd) != 1) {
+ ast_log(LOG_ERROR, "Argument '%s' is not a floating point number for format '%s'\n", arg.var[argcount - 1], formatbuf);
+ goto sprintf_fail;
+ }
+ } else {
+ ast_log(LOG_ERROR, "SPRINTF() has more format specifiers than arguments!\n");
+ goto sprintf_fail;
+ }
+
+ /* Format the argument */
+ snprintf(bufptr, buf + len - bufptr, formatbuf, tmpd);
+
+ /* Update the position of the next parameter to print */
+ bufptr = strchr(buf, '\0');
+ } else if (arg.format[i] == 's') {
+ /* String */
+
+ /* Isolate this format alone */
+ ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
+ formatbuf[&arg.format[i] - formatstart + 1] = '\0';
+
+ /* Format the argument */
+ snprintf(bufptr, buf + len - bufptr, formatbuf, arg.var[argcount++]);
+
+ /* Update the position of the next parameter to print */
+ bufptr = strchr(buf, '\0');
+ } else if (arg.format[i] == '%') {
+ /* Literal data to copy */
+ *bufptr++ = arg.format[i];
+ } else {
+ /* Not supported */
+
+ /* Isolate this format alone */
+ ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
+ formatbuf[&arg.format[i] - formatstart + 1] = '\0';
+
+ ast_log(LOG_ERROR, "Format type not supported: '%s' with argument '%s'\n", formatbuf, arg.var[argcount++]);
+ goto sprintf_fail;
+ }
+ state = -1;
+ break;
+ default:
+ if (arg.format[i] == '%') {
+ state = SPRINTF_FLAG;
+ formatstart = &arg.format[i];
+ break;
+ } else {
+ /* Literal data to copy */
+ *bufptr++ = arg.format[i];
+ }
+ }
+ }
+ *bufptr = '\0';
+ return 0;
+sprintf_fail:
+ return -1;
+}
+
+static struct ast_custom_function sprintf_function = {
+ .name = "SPRINTF",
+ .read = acf_sprintf,
+};
+
+static int unload_module(void)
+{
+ int res = 0;
+
+ res |= ast_custom_function_unregister(&sprintf_function);
+
+ return res;
+}
+
+static int load_module(void)
+{
+ int res = 0;
+
+ res |= ast_custom_function_register(&sprintf_function);
+
+ return res;
+}
+
+AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "SPRINTF dialplan function");
diff --git a/funcs/func_strings.c b/funcs/func_strings.c
index 2e254ab87f..350a5ea198 100644
--- a/funcs/func_strings.c
+++ b/funcs/func_strings.c
@@ -260,25 +260,6 @@ AST_THREADSTORAGE(result_buf);
<para>Example: ${LEN(example)} returns 7</para>
</description>
</function>
- <function name="SPRINTF" language="en_US">
- <synopsis>
- Format a variable according to a format string.
- </synopsis>
- <syntax>
- <parameter name="format" required="true" />
- <parameter name="arg1" required="true" />
- <parameter name="arg2" multiple="true" />
- <parameter name="argN" />
- </syntax>
- <description>
- <para>Parses the format string specified and returns a string matching
- that format. Supports most options found in <emphasis>sprintf(3)</emphasis>.
- Returns a shortened string if a format specifier is not recognized.</para>
- </description>
- <see-also>
- <ref type="manpage">sprintf(3)</ref>
- </see-also>
- </function>
<function name="QUOTE" language="en_US">
<synopsis>
Quotes a given string, escaping embedded quotes as necessary
@@ -744,155 +725,6 @@ static struct ast_custom_function array_function = {
.write = array,
};
-static int acf_sprintf(struct ast_channel *chan, const char *cmd, char *data, char *buf, size_t len)
-{
-#define SPRINTF_FLAG 0
-#define SPRINTF_WIDTH 1
-#define SPRINTF_PRECISION 2
-#define SPRINTF_LENGTH 3
-#define SPRINTF_CONVERSION 4
- int i, state = -1, argcount = 0;
- char *formatstart = NULL, *bufptr = buf;
- char formatbuf[256] = "";
- int tmpi;
- double tmpd;
- AST_DECLARE_APP_ARGS(arg,
- AST_APP_ARG(format);
- AST_APP_ARG(var)[100];
- );
-
- AST_STANDARD_APP_ARGS(arg, data);
-
- /* Scan the format, converting each argument into the requisite format type. */
- for (i = 0; arg.format[i]; i++) {
- switch (state) {
- case SPRINTF_FLAG:
- if (strchr("#0- +'I", arg.format[i]))
- break;
- state = SPRINTF_WIDTH;
- case SPRINTF_WIDTH:
- if (arg.format[i] >= '0' && arg.format[i] <= '9')
- break;
-
- /* Next character must be a period to go into a precision */
- if (arg.format[i] == '.') {
- state = SPRINTF_PRECISION;
- } else {
- state = SPRINTF_LENGTH;
- i--;
- }
- break;
- case SPRINTF_PRECISION:
- if (arg.format[i] >= '0' && arg.format[i] <= '9')
- break;
- state = SPRINTF_LENGTH;
- case SPRINTF_LENGTH:
- if (strchr("hl", arg.format[i])) {
- if (arg.format[i + 1] == arg.format[i])
- i++;
- state = SPRINTF_CONVERSION;
- break;
- } else if (strchr("Lqjzt", arg.format[i])) {
- state = SPRINTF_CONVERSION;
- break;
- }
- state = SPRINTF_CONVERSION;
- case SPRINTF_CONVERSION:
- if (strchr("diouxXc", arg.format[i])) {
- /* Integer */
-
- /* Isolate this format alone */
- ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
- formatbuf[&arg.format[i] - formatstart + 1] = '\0';
-
- /* Convert the argument into the required type */
- if (arg.var[argcount]) {
- if (sscanf(arg.var[argcount++], "%d", &tmpi) != 1) {
- ast_log(LOG_ERROR, "Argument '%s' is not an integer number for format '%s'\n", arg.var[argcount - 1], formatbuf);
- goto sprintf_fail;
- }
- } else {
- ast_log(LOG_ERROR, "SPRINTF() has more format specifiers than arguments!\n");
- goto sprintf_fail;
- }
-
- /* Format the argument */
- snprintf(bufptr, buf + len - bufptr, formatbuf, tmpi);
-
- /* Update the position of the next parameter to print */
- bufptr = strchr(buf, '\0');
- } else if (strchr("eEfFgGaA", arg.format[i])) {
- /* Double */
-
- /* Isolate this format alone */
- ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
- formatbuf[&arg.format[i] - formatstart + 1] = '\0';
-
- /* Convert the argument into the required type */
- if (arg.var[argcount]) {
- if (sscanf(arg.var[argcount++], "%lf", &tmpd) != 1) {
- ast_log(LOG_ERROR, "Argument '%s' is not a floating point number for format '%s'\n", arg.var[argcount - 1], formatbuf);
- goto sprintf_fail;
- }
- } else {
- ast_log(LOG_ERROR, "SPRINTF() has more format specifiers than arguments!\n");
- goto sprintf_fail;
- }
-
- /* Format the argument */
- snprintf(bufptr, buf + len - bufptr, formatbuf, tmpd);
-
- /* Update the position of the next parameter to print */
- bufptr = strchr(buf, '\0');
- } else if (arg.format[i] == 's') {
- /* String */
-
- /* Isolate this format alone */
- ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
- formatbuf[&arg.format[i] - formatstart + 1] = '\0';
-
- /* Format the argument */
- snprintf(bufptr, buf + len - bufptr, formatbuf, arg.var[argcount++]);
-
- /* Update the position of the next parameter to print */
- bufptr = strchr(buf, '\0');
- } else if (arg.format[i] == '%') {
- /* Literal data to copy */
- *bufptr++ = arg.format[i];
- } else {
- /* Not supported */
-
- /* Isolate this format alone */
- ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
- formatbuf[&arg.format[i] - formatstart + 1] = '\0';
-
- ast_log(LOG_ERROR, "Format type not supported: '%s' with argument '%s'\n", formatbuf, arg.var[argcount++]);
- goto sprintf_fail;
- }
- state = -1;
- break;
- default:
- if (arg.format[i] == '%') {
- state = SPRINTF_FLAG;
- formatstart = &arg.format[i];
- break;
- } else {
- /* Literal data to copy */
- *bufptr++ = arg.format[i];
- }
- }
- }
- *bufptr = '\0';
- return 0;
-sprintf_fail:
- return -1;
-}
-
-static struct ast_custom_function sprintf_function = {
- .name = "SPRINTF",
- .read = acf_sprintf,
-};
-
static int quote(struct ast_channel *chan, const char *cmd, char *data, char *buf, size_t len)
{
char *bufptr = buf, *dataptr = data;
@@ -1120,7 +952,6 @@ static int unload_module(void)
res |= ast_custom_function_unregister(&strptime_function);
res |= ast_custom_function_unregister(&eval_function);
res |= ast_custom_function_unregister(&keypadhash_function);
- res |= ast_custom_function_unregister(&sprintf_function);
res |= ast_custom_function_unregister(&hashkeys_function);
res |= ast_custom_function_unregister(&hash_function);
res |= ast_unregister_application(app_clearhash);
@@ -1145,7 +976,6 @@ static int load_module(void)
res |= ast_custom_function_register(&strptime_function);
res |= ast_custom_function_register(&eval_function);
res |= ast_custom_function_register(&keypadhash_function);
- res |= ast_custom_function_register(&sprintf_function);
res |= ast_custom_function_register(&hashkeys_function);
res |= ast_custom_function_register(&hash_function);
res |= ast_register_application_xml(app_clearhash, exec_clearhash);
diff --git a/main/Makefile b/main/Makefile
index 51acf20eec..56b964d161 100644
--- a/main/Makefile
+++ b/main/Makefile
@@ -142,7 +142,7 @@ ifneq ($(findstring ENABLE_UPLOADS,$(MENUSELECT_CFLAGS)),)
http.o: ASTCFLAGS+=$(GMIME_INCLUDE)
endif
-stdtime/localtime.o: ASTCFLAGS+=$(AST_NO_STRICT_OVERFLOW)
+stdtime/localtime.o: ASTCFLAGS+=$(AST_NO_STRICT_OVERFLOW) -Wno-format-nonliteral
AST_EMBED_LDSCRIPTS:=$(sort $(EMBED_LDSCRIPTS))
AST_EMBED_LDFLAGS:=$(foreach dep,$(EMBED_LDFLAGS),$(value $(dep)))
diff --git a/res/res_config_sqlite.c b/res/res_config_sqlite.c
index ff3c416cc2..5f72abc9a1 100644
--- a/res/res_config_sqlite.c
+++ b/res/res_config_sqlite.c
@@ -555,8 +555,7 @@ static char *sql_create_cdr_table =
/*!
* SQL query format to describe the table structure
*/
-static char *sql_table_structure =
-"SELECT sql FROM sqlite_master WHERE type='table' AND tbl_name='%s'";
+#define sql_table_structure "SELECT sql FROM sqlite_master WHERE type='table' AND tbl_name='%s'"
/*!
* SQL query format to fetch the static configuration of a file.
@@ -564,11 +563,11 @@ static char *sql_table_structure =
*
* \see add_cfg_entry()
*/
-static const char *sql_get_config_table =
-"SELECT *"
-" FROM '%q'"
-" WHERE filename = '%q' AND commented = 0"
-" ORDER BY cat_metric ASC, var_metric ASC;";
+#define sql_get_config_table \
+ "SELECT *" \
+ " FROM '%q'" \
+ " WHERE filename = '%q' AND commented = 0" \
+ " ORDER BY cat_metric ASC, var_metric ASC;"
static void free_table(struct sqlite_cache_tables *tblptr)
{
--
GitLab