Fossil SCM

An alternate approach to [ae8fc0e0b5e6] which instead rejects all GET and COOKIE values which, after decoding, contain any control characters. We have(?) no(?) use cases where control characters are legitimately needed for GET/COOKIE values.

stephan 2025-09-01 15:27 trunk
Commit c61ae84cabe665acd7dd2a062bd9da2a1af94b6808324701112fa3530517809b
Parent 55c972103ffc130…
1 file changed +22 -5
~ src/cgi.c
M src/cgi.c
+22 -5
--- src/cgi.c
+++ src/cgi.c
@@ -944,10 +944,20 @@
944944
 ** portion is fixed but a copy is be made of zValue.
945945
 */
946946
 void cgi_setenv(const char *zName, const char *zValue){
947947
 cgi_set_parameter_nocopy(zName, fossil_strdup(zValue), 0);
948948
 }
949
+
950
+/*
951
+** Returns true if NUL-terminated z contains any non-NUL
952
+** control characters (<0x20, 32d).
953
+*/
954
+static int contains_ctrl(const char *z){
955
+ assert(z);
956
+ for( ; *z>=0x20; ++z ){}
957
+ return 0!=*z;
958
+}
949959
950960
 /*
951961
 ** Add a list of query parameters or cookies to the parameter set.
952962
 **
953963
 ** Each parameter is of the form NAME=VALUE. Both the NAME and the
@@ -974,12 +984,16 @@
974984
 ** before the NAME is ignored.
975985
 **
976986
 ** The input string "z" is modified but no copies is made. "z"
977987
 ** should not be deallocated or changed again after this routine
978988
 ** returns or it will corrupt the parameter table.
989
+**
990
+** If bPermitCtrl is false and the decoded value of any entry in z
991
+** contains control characters (<0x20, 32d) then that key/value pair
992
+** are skipped.
979993
 */
980
-static void add_param_list(char *z, int terminator){
994
+static void add_param_list(char *z, int terminator, int bPermitCtrl){
981995
 int isQP = terminator=='&';
982996
 while( *z ){
983997
 char *zName;
984998
 char *zValue;
985999
 while( fossil_isspace(*z) ){ z++; }
@@ -998,11 +1012,14 @@
9981012
 }else{
9991013
 if( *z ){ *z++ = 0; }
10001014
 zValue = "";
10011015
 }
10021016
 if( zName[0] && fossil_no_strange_characters(zName+1) ){
1003
- if( fossil_islower(zName[0]) ){
1017
+ if( 0==bPermitCtrl && contains_ctrl(zValue) ){
1018
+ continue /* Reject it. An argument could be made
1019
+ ** for break instead of continue. */;
1020
+ }else if( fossil_islower(zName[0]) ){
10041021
 cgi_set_parameter_nocopy(zName, zValue, isQP);
10051022
 }else if( fossil_isupper(zName[0]) ){
10061023
 cgi_set_parameter_nocopy_tolower(zName, zValue, isQP);
10071024
 }
10081025
 }
@@ -1297,11 +1314,11 @@
12971314
 int rc = 0;
12981315
 char * z = (char*)P("QUERY_STRING");
12991316
 if( z ){
13001317
 rc = 0x01;
13011318
 z = fossil_strdup(z);
1302
- add_param_list(z, '&');
1319
+ add_param_list(z, '&', 0);
13031320
 z = (char*)P("skin");
13041321
 if( z ){
13051322
 char *zErr = skin_use_alternative(z, 2, SKIN_FROM_QPARAM);
13061323
 rc |= 0x02;
13071324
 if( !zErr && P("once")==0 ){
@@ -1457,11 +1474,11 @@
14571474
 }
14581475
 #endif
14591476
 z = (char*)P("HTTP_COOKIE");
14601477
 if( z ){
14611478
 z = fossil_strdup(z);
1462
- add_param_list(z, ';');
1479
+ add_param_list(z, ';', 0);
14631480
 z = (char*)cookie_value("skin",0);
14641481
 if(z){
14651482
 skin_use_alternative(z, 2, SKIN_FROM_COOKIE);
14661483
 }
14671484
 }
@@ -1520,11 +1537,11 @@
15201537
 || fossil_strncmp(g.zContentType,"multipart/form-data",19)==0
15211538
 ){
15221539
 char *z = blob_str(&g.cgiIn);
15231540
 cgi_trace(z);
15241541
 if( g.zContentType[0]=='a' ){
1525
- add_param_list(z, '&');
1542
+ add_param_list(z, '&', 1);
15261543
 }else{
15271544
 process_multipart_form_data(z, len);
15281545
 }
15291546
 blob_init(&g.cgiIn, 0, 0);
15301547
 }
15311548
--- src/cgi.c
+++ src/cgi.c
@@ -944,10 +944,20 @@
944 ** portion is fixed but a copy is be made of zValue.
945 */
946 void cgi_setenv(const char *zName, const char *zValue){
947 cgi_set_parameter_nocopy(zName, fossil_strdup(zValue), 0);
948 }
 
 
 
 
 
 
 
 
 
 
949
950 /*
951 ** Add a list of query parameters or cookies to the parameter set.
952 **
953 ** Each parameter is of the form NAME=VALUE. Both the NAME and the
@@ -974,12 +984,16 @@
974 ** before the NAME is ignored.
975 **
976 ** The input string "z" is modified but no copies is made. "z"
977 ** should not be deallocated or changed again after this routine
978 ** returns or it will corrupt the parameter table.
 
 
 
 
979 */
980 static void add_param_list(char *z, int terminator){
981 int isQP = terminator=='&';
982 while( *z ){
983 char *zName;
984 char *zValue;
985 while( fossil_isspace(*z) ){ z++; }
@@ -998,11 +1012,14 @@
998 }else{
999 if( *z ){ *z++ = 0; }
1000 zValue = "";
1001 }
1002 if( zName[0] && fossil_no_strange_characters(zName+1) ){
1003 if( fossil_islower(zName[0]) ){
 
 
 
1004 cgi_set_parameter_nocopy(zName, zValue, isQP);
1005 }else if( fossil_isupper(zName[0]) ){
1006 cgi_set_parameter_nocopy_tolower(zName, zValue, isQP);
1007 }
1008 }
@@ -1297,11 +1314,11 @@
1297 int rc = 0;
1298 char * z = (char*)P("QUERY_STRING");
1299 if( z ){
1300 rc = 0x01;
1301 z = fossil_strdup(z);
1302 add_param_list(z, '&');
1303 z = (char*)P("skin");
1304 if( z ){
1305 char *zErr = skin_use_alternative(z, 2, SKIN_FROM_QPARAM);
1306 rc |= 0x02;
1307 if( !zErr && P("once")==0 ){
@@ -1457,11 +1474,11 @@
1457 }
1458 #endif
1459 z = (char*)P("HTTP_COOKIE");
1460 if( z ){
1461 z = fossil_strdup(z);
1462 add_param_list(z, ';');
1463 z = (char*)cookie_value("skin",0);
1464 if(z){
1465 skin_use_alternative(z, 2, SKIN_FROM_COOKIE);
1466 }
1467 }
@@ -1520,11 +1537,11 @@
1520 || fossil_strncmp(g.zContentType,"multipart/form-data",19)==0
1521 ){
1522 char *z = blob_str(&g.cgiIn);
1523 cgi_trace(z);
1524 if( g.zContentType[0]=='a' ){
1525 add_param_list(z, '&');
1526 }else{
1527 process_multipart_form_data(z, len);
1528 }
1529 blob_init(&g.cgiIn, 0, 0);
1530 }
1531
--- src/cgi.c
+++ src/cgi.c
@@ -944,10 +944,20 @@
944 ** portion is fixed but a copy is be made of zValue.
945 */
946 void cgi_setenv(const char *zName, const char *zValue){
947 cgi_set_parameter_nocopy(zName, fossil_strdup(zValue), 0);
948 }
949
950 /*
951 ** Returns true if NUL-terminated z contains any non-NUL
952 ** control characters (<0x20, 32d).
953 */
954 static int contains_ctrl(const char *z){
955 assert(z);
956 for( ; *z>=0x20; ++z ){}
957 return 0!=*z;
958 }
959
960 /*
961 ** Add a list of query parameters or cookies to the parameter set.
962 **
963 ** Each parameter is of the form NAME=VALUE. Both the NAME and the
@@ -974,12 +984,16 @@
984 ** before the NAME is ignored.
985 **
986 ** The input string "z" is modified but no copies is made. "z"
987 ** should not be deallocated or changed again after this routine
988 ** returns or it will corrupt the parameter table.
989 **
990 ** If bPermitCtrl is false and the decoded value of any entry in z
991 ** contains control characters (<0x20, 32d) then that key/value pair
992 ** are skipped.
993 */
994 static void add_param_list(char *z, int terminator, int bPermitCtrl){
995 int isQP = terminator=='&';
996 while( *z ){
997 char *zName;
998 char *zValue;
999 while( fossil_isspace(*z) ){ z++; }
@@ -998,11 +1012,14 @@
1012 }else{
1013 if( *z ){ *z++ = 0; }
1014 zValue = "";
1015 }
1016 if( zName[0] && fossil_no_strange_characters(zName+1) ){
1017 if( 0==bPermitCtrl && contains_ctrl(zValue) ){
1018 continue /* Reject it. An argument could be made
1019 ** for break instead of continue. */;
1020 }else if( fossil_islower(zName[0]) ){
1021 cgi_set_parameter_nocopy(zName, zValue, isQP);
1022 }else if( fossil_isupper(zName[0]) ){
1023 cgi_set_parameter_nocopy_tolower(zName, zValue, isQP);
1024 }
1025 }
@@ -1297,11 +1314,11 @@
1314 int rc = 0;
1315 char * z = (char*)P("QUERY_STRING");
1316 if( z ){
1317 rc = 0x01;
1318 z = fossil_strdup(z);
1319 add_param_list(z, '&', 0);
1320 z = (char*)P("skin");
1321 if( z ){
1322 char *zErr = skin_use_alternative(z, 2, SKIN_FROM_QPARAM);
1323 rc |= 0x02;
1324 if( !zErr && P("once")==0 ){
@@ -1457,11 +1474,11 @@
1474 }
1475 #endif
1476 z = (char*)P("HTTP_COOKIE");
1477 if( z ){
1478 z = fossil_strdup(z);
1479 add_param_list(z, ';', 0);
1480 z = (char*)cookie_value("skin",0);
1481 if(z){
1482 skin_use_alternative(z, 2, SKIN_FROM_COOKIE);
1483 }
1484 }
@@ -1520,11 +1537,11 @@
1537 || fossil_strncmp(g.zContentType,"multipart/form-data",19)==0
1538 ){
1539 char *z = blob_str(&g.cgiIn);
1540 cgi_trace(z);
1541 if( g.zContentType[0]=='a' ){
1542 add_param_list(z, '&', 1);
1543 }else{
1544 process_multipart_form_data(z, len);
1545 }
1546 blob_init(&g.cgiIn, 0, 0);
1547 }
1548

Keyboard Shortcuts

Open search /
Next entry (timeline) j
Previous entry (timeline) k
Open focused entry Enter
Show this help ?
Toggle theme Top nav button