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
1.6.2
Kevin P. Fleming 17 years ago
parent 445c5296da
commit 9a7c28cd5a

@ -237,7 +237,7 @@ endif
ASTCFLAGS+=-Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations $(DEBUG) ASTCFLAGS+=-Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations $(DEBUG)
ifeq ($(AST_DEVMODE),yes) 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 endif
ifneq ($(findstring BSD,$(OSARCH)),) ifneq ($(findstring BSD,$(OSARCH)),)

@ -85,7 +85,7 @@ From 1.6.1 to 1.6.2:
has been replaced with 'UNSUPPORTED'). This change makes the has been replaced with 'UNSUPPORTED'). This change makes the
SendImage application more consistent with other applications. 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 Please have a look at configs/skinny.conf.sample and update
your skinny.conf. your skinny.conf.
@ -95,3 +95,8 @@ From 1.6.1 to 1.6.2:
case-insensitive comparisons now when originally hashing based on case-insensitive comparisons now when originally hashing based on
queue names, meaning that now the two queues mentioned as examples queue names, meaning that now the two queues mentioned as examples
earlier will be seen as having the same name. 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.

@ -1347,7 +1347,7 @@ static void enc_ie_useruser(unsigned char **ntmode, msg_t *msg, int protocol, ch
i = 0; i = 0;
while(i < user_len) 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++; i++;
} }
@ -1393,7 +1393,7 @@ static void dec_ie_useruser(unsigned char *p, Q931_info_t *qi, int *protocol, ch
i = 0; i = 0;
while(i < *user_len) 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++; i++;
} }
debug[i*3] = '\0'; debug[i*3] = '\0';

@ -882,12 +882,14 @@ static int _parse (union misdn_cfg_pt *dest, const char *value, enum misdn_cfg_t
break; break;
case MISDN_CTYPE_INT: case MISDN_CTYPE_INT:
{ {
char *pat; int res;
if (strchr(value,'x'))
pat="%x"; if (strchr(value,'x')) {
else res = sscanf(value, "%x", &tmp);
pat="%d"; } else {
if (sscanf(value, pat, &tmp)) { res = sscanf(value, "%d", &tmp);
}
if (res) {
dest->num = ast_malloc(sizeof(int)); dest->num = ast_malloc(sizeof(int));
memcpy(dest->num, &tmp, sizeof(int)); memcpy(dest->num, &tmp, sizeof(int));
} else } else

@ -18,3 +18,10 @@ MENUSELECT_DESCRIPTION=Dialplan Functions
all: _all all: _all
include $(ASTTOPDIR)/Makefile.moddir_rules 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

@ -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");

@ -260,25 +260,6 @@ AST_THREADSTORAGE(result_buf);
<para>Example: ${LEN(example)} returns 7</para> <para>Example: ${LEN(example)} returns 7</para>
</description> </description>
</function> </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"> <function name="QUOTE" language="en_US">
<synopsis> <synopsis>
Quotes a given string, escaping embedded quotes as necessary Quotes a given string, escaping embedded quotes as necessary
@ -744,155 +725,6 @@ static struct ast_custom_function array_function = {
.write = array, .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) static int quote(struct ast_channel *chan, const char *cmd, char *data, char *buf, size_t len)
{ {
char *bufptr = buf, *dataptr = data; 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(&strptime_function);
res |= ast_custom_function_unregister(&eval_function); res |= ast_custom_function_unregister(&eval_function);
res |= ast_custom_function_unregister(&keypadhash_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(&hashkeys_function);
res |= ast_custom_function_unregister(&hash_function); res |= ast_custom_function_unregister(&hash_function);
res |= ast_unregister_application(app_clearhash); 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(&strptime_function);
res |= ast_custom_function_register(&eval_function); res |= ast_custom_function_register(&eval_function);
res |= ast_custom_function_register(&keypadhash_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(&hashkeys_function);
res |= ast_custom_function_register(&hash_function); res |= ast_custom_function_register(&hash_function);
res |= ast_register_application_xml(app_clearhash, exec_clearhash); res |= ast_register_application_xml(app_clearhash, exec_clearhash);

@ -142,7 +142,7 @@ ifneq ($(findstring ENABLE_UPLOADS,$(MENUSELECT_CFLAGS)),)
http.o: ASTCFLAGS+=$(GMIME_INCLUDE) http.o: ASTCFLAGS+=$(GMIME_INCLUDE)
endif 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_LDSCRIPTS:=$(sort $(EMBED_LDSCRIPTS))
AST_EMBED_LDFLAGS:=$(foreach dep,$(EMBED_LDFLAGS),$(value $(dep))) AST_EMBED_LDFLAGS:=$(foreach dep,$(EMBED_LDFLAGS),$(value $(dep)))

@ -555,8 +555,7 @@ static char *sql_create_cdr_table =
/*! /*!
* SQL query format to describe the table structure * SQL query format to describe the table structure
*/ */
static char *sql_table_structure = #define sql_table_structure "SELECT sql FROM sqlite_master WHERE type='table' AND tbl_name='%s'"
"SELECT sql FROM sqlite_master WHERE type='table' AND tbl_name='%s'";
/*! /*!
* SQL query format to fetch the static configuration of a file. * SQL query format to fetch the static configuration of a file.
@ -564,11 +563,11 @@ static char *sql_table_structure =
* *
* \see add_cfg_entry() * \see add_cfg_entry()
*/ */
static const char *sql_get_config_table = #define sql_get_config_table \
"SELECT *" "SELECT *" \
" FROM '%q'" " FROM '%q'" \
" WHERE filename = '%q' AND commented = 0" " WHERE filename = '%q' AND commented = 0" \
" ORDER BY cat_metric ASC, var_metric ASC;"; " ORDER BY cat_metric ASC, var_metric ASC;"
static void free_table(struct sqlite_cache_tables *tblptr) static void free_table(struct sqlite_cache_tables *tblptr)
{ {

Loading…
Cancel
Save