Skip to content

Commit

Permalink
Merge pull request MapServer#7118 from rouault/tostring_validation
Browse files Browse the repository at this point in the history 
tostring() expression function: validate format string, and make sure buffer is large enough
  • Loading branch information
@rouault
rouault authored Jul 21, 2024
2 parents f1007c9 + eb64cc1 commit 27b78a7
Show file tree
Hide file tree
Showing 6 changed files with 170 additions and 25 deletions.
1 change: 0 additions & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1163,7 +1163,6 @@ option(BUILD_TESTING "Build unit test" ON)
if(BUILD_DYNAMIC AND BUILD_TESTING)
enable_testing()
add_executable(unit_test tests/unit/test.cpp)
target_include_directories(unit_test PRIVATE src)
target_link_libraries(unit_test PRIVATE mapserver)
add_test(NAME unit_test COMMAND unit_test)
endif()
43 changes: 23 additions & 20 deletions src/mapparser.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -557,8 +557,8 @@ static const yytype_uint16 yyrline[] =
672, 689, 705, 723, 724, 725, 726, 727, 728, 729,
736, 737, 738, 739, 750, 751, 754, 755, 756, 770,
784, 798, 812, 826, 840, 854, 868, 882, 896, 910,
924, 939, 961, 962, 963, 969, 974, 978, 982, 986,
990, 996, 997
924, 939, 961, 962, 963, 969, 978, 982, 986, 990,
994, 1000, 1001
};
#endif

Expand Down Expand Up @@ -2892,66 +2892,70 @@ YYSTYPE yylval YY_INITIAL_VALUE (= yyval_default);
case 95:
#line 969 "mapparser.y" /* yacc.c:1646 */
{
(yyval.strval) = (char *) malloc(strlen((yyvsp[-1].strval)) + 64); /* Plenty big? Should use snprintf below... */
sprintf((yyval.strval), (yyvsp[-1].strval), (yyvsp[-3].dblval));
char* ret = msToString((yyvsp[-1].strval), (yyvsp[-3].dblval));
msReplaceFreeableStr(&((yyvsp[-1].strval)), NULL);
if(!ret) {
yyerror(p, "tostring() failed.");
return(-1);
}
(yyval.strval) = ret;
}
#line 2900 "/vagrant/mapparser.c" /* yacc.c:1646 */
#line 2904 "/vagrant/mapparser.c" /* yacc.c:1646 */
break;

case 96:
#line 974 "mapparser.y" /* yacc.c:1646 */
#line 978 "mapparser.y" /* yacc.c:1646 */
{
(yyvsp[-1].strval) = msCommifyString((yyvsp[-1].strval));
(yyval.strval) = (yyvsp[-1].strval);
}
#line 2909 "/vagrant/mapparser.c" /* yacc.c:1646 */
#line 2913 "/vagrant/mapparser.c" /* yacc.c:1646 */
break;

case 97:
#line 978 "mapparser.y" /* yacc.c:1646 */
#line 982 "mapparser.y" /* yacc.c:1646 */
{
msStringToUpper((yyvsp[-1].strval));
(yyval.strval) = (yyvsp[-1].strval);
}
#line 2918 "/vagrant/mapparser.c" /* yacc.c:1646 */
#line 2922 "/vagrant/mapparser.c" /* yacc.c:1646 */
break;

case 98:
#line 982 "mapparser.y" /* yacc.c:1646 */
#line 986 "mapparser.y" /* yacc.c:1646 */
{
msStringToLower((yyvsp[-1].strval));
(yyval.strval) = (yyvsp[-1].strval);
}
#line 2927 "/vagrant/mapparser.c" /* yacc.c:1646 */
#line 2931 "/vagrant/mapparser.c" /* yacc.c:1646 */
break;

case 99:
#line 986 "mapparser.y" /* yacc.c:1646 */
#line 990 "mapparser.y" /* yacc.c:1646 */
{
msStringInitCap((yyvsp[-1].strval));
(yyval.strval) = (yyvsp[-1].strval);
}
#line 2936 "/vagrant/mapparser.c" /* yacc.c:1646 */
#line 2940 "/vagrant/mapparser.c" /* yacc.c:1646 */
break;

case 100:
#line 990 "mapparser.y" /* yacc.c:1646 */
#line 994 "mapparser.y" /* yacc.c:1646 */
{
msStringFirstCap((yyvsp[-1].strval));
(yyval.strval) = (yyvsp[-1].strval);
}
#line 2945 "/vagrant/mapparser.c" /* yacc.c:1646 */
#line 2949 "/vagrant/mapparser.c" /* yacc.c:1646 */
break;

case 102:
#line 997 "mapparser.y" /* yacc.c:1646 */
#line 1001 "mapparser.y" /* yacc.c:1646 */
{ (yyval.tmval) = (yyvsp[-1].tmval); }
#line 2951 "/vagrant/mapparser.c" /* yacc.c:1646 */
#line 2955 "/vagrant/mapparser.c" /* yacc.c:1646 */
break;


#line 2955 "/vagrant/mapparser.c" /* yacc.c:1646 */
#line 2959 "/vagrant/mapparser.c" /* yacc.c:1646 */
default: break;
}
/* User semantic actions sometimes alter yychar, and that requires
Expand Down Expand Up @@ -3000,7 +3004,6 @@ YYSTYPE yylval YY_INITIAL_VALUE (= yyval_default);
if (!yyerrstatus)
{
++yynerrs;
if(0) printf("%d", yynerrs); /* avoid warning about unused yynerrs */
#if ! YYERROR_VERBOSE
yyerror (p, YY_("syntax error"));
#else
Expand Down Expand Up @@ -3180,7 +3183,7 @@ YYSTYPE yylval YY_INITIAL_VALUE (= yyval_default);
#endif
return yyresult;
}
#line 1000 "mapparser.y" /* yacc.c:1906 */
#line 1004 "mapparser.y" /* yacc.c:1906 */


/*
Expand Down
8 changes: 6 additions & 2 deletions src/mapparser.y
View file
Original file line number Diff line number Diff line change
Expand Up @@ -967,9 +967,13 @@ string_exp: STRING
msReplaceFreeableStr(&($3), NULL);
}
| TOSTRING '(' math_exp ',' string_exp ')' {
$$ = (char *) malloc(strlen($5) + 64); /* Plenty big? Should use snprintf below... */
sprintf($$, $5, $3);
char* ret = msToString($5, $3);
msReplaceFreeableStr(&($5), NULL);
if(!ret) {
yyerror(p, "tostring() failed.");
return(-1);
}
$$ = ret;
}
| COMMIFY '(' string_exp ')' {
$3 = msCommifyString($3);
Expand Down
1 change: 1 addition & 0 deletions src/mapserver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2879,6 +2879,7 @@ MS_DLL_EXPORT char *msJoinStrings(char **array, int arrayLength,
const char *delimiter);
MS_DLL_EXPORT char *msHashString(const char *pszStr);
MS_DLL_EXPORT char *msCommifyString(char *str);
MS_DLL_EXPORT char *msToString(const char *format, double value);
MS_DLL_EXPORT int msHexToInt(char *hex);
MS_DLL_EXPORT char *msGetEncodedString(const char *string,
const char *encoding);
Expand Down
63 changes: 63 additions & 0 deletions src/mapstring.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "cpl_vsi.h"

#include <ctype.h>
#include <float.h>
#include <string.h>
#include <errno.h>

Expand Down Expand Up @@ -1548,6 +1549,68 @@ char *msCommifyString(char *str) {
return str;
}

/************************************************************************/
/* msToString() */
/************************************************************************/

char *msToString(const char *format, double value) {
bool pctAlreadyFound = false;
// Validate that the formatting string is OK for a single input double value
int extra_size = 0;
for (const char *ptr = format; *ptr; ++ptr) {
if (*ptr == '%' && ptr[1] == '%') {
++ptr;
} else if (*ptr == '%') {
if (pctAlreadyFound) {
msSetError(MS_MISCERR, "More than one conversion specifier",
"msToString()");
return nullptr;
}
pctAlreadyFound = true;
++ptr;
// Skip flag characters
while (*ptr == '+' || *ptr == '-' || *ptr == ' ' || *ptr == '\'' ||
*ptr == '0') {
++ptr;
}
// Skip width
if (*ptr >= '1' && *ptr <= '9') {
extra_size = atoi(ptr);
do {
++ptr;
} while (*ptr >= '0' && *ptr <= '9');
if (extra_size > 1024) {
// To avoid arbitrary memory allocatin
msSetError(MS_MISCERR, "Too large width", "msToString()");
return nullptr;
}
}
// maximum double value is of the order of ~1e308
if (extra_size < DBL_MAX_10_EXP)
extra_size = DBL_MAX_10_EXP;
extra_size += 32; // extra margin

// Skip precision
if (*ptr == '.') {
++ptr;
while (*ptr >= '0' && *ptr <= '9')
++ptr;
}
// Check conversion specifier
if (!(*ptr == 'e' || *ptr == 'E' || *ptr == 'f' || *ptr == 'F' ||
*ptr == 'g' || *ptr == 'G')) {
msSetError(MS_MISCERR, "Invalid conversion specifier", "msToString()");
return nullptr;
}
}
}
// extra_size / 3 if thousands' grouping characters is used
const size_t nBufferSize = strlen(format) + extra_size + (extra_size / 3) + 1;
char *ret = static_cast<char *>(msSmallMalloc(nBufferSize));
snprintf(ret, nBufferSize, format, value);
return ret;
}

/* -------------------------------------------------------------------------------
*/
/* Replace all occurrences of old with new in str. */
Expand Down
79 changes: 77 additions & 2 deletions tests/unit/test.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#include "mapserver.h"
#include "maperror.h"
#include "../../src/mapserver.h"
#include "../../src/maperror.h"

/* ----------------------------------------------------------------------- */

Expand All @@ -19,6 +19,17 @@ static void EXPECT_STREQ_func(const char *got, const char *expected,
#define EXPECT_STREQ(got, expected) \
EXPECT_STREQ_func(got, expected, __func__, __LINE__)

static void EXPECT_TRUE_FUNC(bool cond, const char *condstr,
const char *function, int line) {
if (!cond) {
fprintf(stderr, "EXPECT_TRUE(\"%s\") failed at %s:%d\n", condstr, function,
line);
gTestRetCode = 1;
}
}

#define EXPECT_TRUE(cond) EXPECT_TRUE_FUNC(cond, #cond, __func__, __LINE__)

/* ----------------------------------------------------------------------- */

static void testRedactCredentials() {
Expand Down Expand Up @@ -68,7 +79,71 @@ static void testRedactCredentials() {

/* ----------------------------------------------------------------------- */

static void testToString() {
{
char *ret = msToString("x%f %%y", 1.5);
EXPECT_STREQ(ret, "x1.500000 %y");
msFree(ret);
}
{
char *ret = msToString("%+f", 1.5);
EXPECT_STREQ(ret, "+1.500000");
msFree(ret);
}
{
char *ret = msToString("%5.2f", 1.5);
EXPECT_STREQ(ret, " 1.50");
msFree(ret);
}
{
char *ret = msToString("%f", 1e308);
EXPECT_STREQ(
ret, "10000000000000000109790636294404554174049230967731184633681068290"
"31575854049114915371633289784946888990612496697211725156115902837"
"43140088328307009198146046031271664502933027185697489699588559043"
"33838446616500117842689762621294517762809119578670745812278397017"
"1784415105291802893207873272974885715430223118336.000000");
msFree(ret);
}
{
char *ret = msToString("%1f", 1e308);
EXPECT_STREQ(
ret, "10000000000000000109790636294404554174049230967731184633681068290"
"31575854049114915371633289784946888990612496697211725156115902837"
"43140088328307009198146046031271664502933027185697489699588559043"
"33838446616500117842689762621294517762809119578670745812278397017"
"1784415105291802893207873272974885715430223118336.000000");
msFree(ret);
}
{
char *ret = msToString("%320f", 1e308);
EXPECT_STREQ(
ret, " "
"10000000000000000109790636294404554174049230967731184633681068290"
"31575854049114915371633289784946888990612496697211725156115902837"
"43140088328307009198146046031271664502933027185697489699588559043"
"33838446616500117842689762621294517762809119578670745812278397017"
"1784415105291802893207873272974885715430223118336.000000");
msFree(ret);
}
{
char *ret = msToString("%f%f", 1);
EXPECT_TRUE(ret == nullptr);
}
{
char *ret = msToString("%s", 1);
EXPECT_TRUE(ret == nullptr);
}
{
char *ret = msToString("%100000f", 1);
EXPECT_TRUE(ret == nullptr);
}
}

/* ----------------------------------------------------------------------- */

int main() {
testRedactCredentials();
testToString();
return gTestRetCode;
}

0 comments on commit 27b78a7

Please sign in to comment.