Fossil SCM

Improvements to command-string sanitizing and the the sanitizer test command.

drh 2020-06-09 17:32 safe-fossil-system-test
Commit e3185aee7f33384ec2afe8275d810aee05e6253ea052e563f33a77748216771d
Parent 9c38a004adefea0…
1 file changed +50 -12
~ src/util.c
M src/util.c
+50 -12
--- src/util.c
+++ src/util.c
@@ -156,10 +156,17 @@
156156
 }
157157
 }
158158
 return zStart;
159159
 }
160160
161
+/*
162
+** If this local variable is set, fossil_assert_safe_command_string()
163
+** returns false on an unsafe command-string rather than abort. Set
164
+** this variable for testing.
165
+*/
166
+static int safeCmdStrTest = 0;
167
+
161168
 /*
162169
 ** Check the input string to ensure that it is safe to pass into system().
163170
 ** A string is unsafe for system() on unix if it contains any of the following:
164171
 **
165172
 ** * Any occurrance of '$' or '`' except after \
@@ -171,30 +178,30 @@
171178
 ** This routine is intended as a second line of defense against attack.
172179
 ** It should never fail. Dangerous shell strings should be detected and
173180
 ** fixed before calling fossil_system(). This routine serves only as a
174181
 ** safety net in case of bugs elsewhere in the system.
175182
 **
176
-** If an unsafe string is seen, the process aborts.
183
+** If an unsafe string is seen, either abort or return false.
177184
 */
178
-void fossil_assert_safe_command_string(const char *z){
185
+static int fossil_assert_safe_command_string(const char *z){
179186
 int unsafe = 0;
180187
 #ifndef _WIN32
181188
 /* Unix */
182189
 int inQuote = 0;
183190
 int i, c;
184
- for(i=0; (c = z[i])!=0; i++){
191
+ for(i=0; !unsafe && (c = z[i])!=0; i++){
185192
 switch( c ){
186193
 case '$':
187194
 case '`': {
188
- unsafe = i+1;
195
+ if( inQuote!='\'' ) unsafe = i+1;
189196
 break;
190197
 }
191198
 case ';':
192199
 case '|':
193200
 case '&':
194201
 case '\n': {
195
- if( inQuote==0 && z[i+1]!=0 ) unsafe = i+1;
202
+ if( inQuote!='\'' && z[i+1]!=0 ) unsafe = i+1;
196203
 break;
197204
 }
198205
 case '"':
199206
 case '\'': {
200207
 if( inQuote==0 ){
@@ -205,25 +212,52 @@
205212
 break;
206213
 }
207214
 case '\\': {
208215
 if( z[i+1]==0 ){
209216
 unsafe = i+1;
210
- }else{
217
+ }else if( inQuote!='\'' ){
211218
 i++;
212219
 }
213220
 break;
214221
 }
215222
 }
216223
 }
224
+ if( inQuote ) unsafe = i;
217225
 #else
218226
 /* Windows */
219
-
227
+ int i, c;
228
+ int inQuote = 0;
229
+ for(i=0; !unsafe && (c = z[i])!=0; i++){
230
+ switch( c ){
231
+ case '|':
232
+ case '&':
233
+ case '\n': {
234
+ if( inQuote==0 && z[i+1]!=0 ) unsafe = i+1;
235
+ break;
236
+ }
237
+ case '"': {
238
+ if( inQuote==c ){
239
+ inQuote = 0;
240
+ }else{
241
+ inQuote = c;
242
+ }
243
+ break;
244
+ }
245
+ }
246
+ }
247
+ if( inQuote ) unsafe = i;
220248
 #endif
221249
 if( unsafe ){
222
- fossil_fatal("Unsafe command string: %s\n%*shere ----^",
223
- z, unsafe+13, "");
250
+ char *zMsg = mprintf("Unsafe command string: %s\n%*shere ----^",
251
+ z, unsafe+13, "");
252
+ if( safeCmdStrTest ){
253
+ fossil_print("%z\n", zMsg);
254
+ }else{
255
+ fossil_panic("%s", zMsg);
256
+ }
224257
 }
258
+ return !unsafe;
225259
 }
226260
227261
 /*
228262
 ** This function implements a cross-platform "system()" interface.
229263
 */
@@ -236,19 +270,20 @@
236270
 char *zNewCmd = mprintf("\"%s\"", zOrigCmd);
237271
 wchar_t *zUnicode = fossil_utf8_to_unicode(zNewCmd);
238272
 if( g.fSystemTrace ) {
239273
 fossil_trace("SYSTEM: %s\n", zNewCmd);
240274
 }
241
- fossil_assert_safe_command_string(zOrigCmd);
242
- rc = _wsystem(zUnicode);
275
+ if( fossil_assert_safe_command_string(zOrigCmd) ){
276
+ rc = _wsystem(zUnicode);
277
+ }
243278
 fossil_unicode_free(zUnicode);
244279
 free(zNewCmd);
245280
 #else
246281
 /* On unix, evaluate the command directly.
247282
 */
248283
 if( g.fSystemTrace ) fprintf(stderr, "SYSTEM: %s\n", zOrigCmd);
249
- fossil_assert_safe_command_string(zOrigCmd);
284
+ if( !fossil_assert_safe_command_string(zOrigCmd) ) return 1;
250285
251286
 /* Unix systems should never shell-out while processing an HTTP request,
252287
 ** either via CGI, SCGI, or direct HTTP. The following assert verifies
253288
 ** this. And the following assert proves that Fossil is not vulnerable
254289
 ** to the ShellShock or BashDoor bug.
@@ -265,13 +300,16 @@
265300
266301
 /*
267302
 ** COMMAND: test-fossil-system
268303
 **
269304
 ** Read lines of input and send them to fossil_system() for evaluation.
305
+** Use this command to verify that fossil_system() will not run "unsafe"
306
+** commands.
270307
 */
271308
 void test_fossil_system_cmd(void){
272309
 char zLine[10000];
310
+ safeCmdStrTest = 1;
273311
 while(1){
274312
 size_t n;
275313
 printf("system-test> ");
276314
 fflush(stdout);
277315
 if( !fgets(zLine, sizeof(zLine), stdin) ) break;
278316
--- src/util.c
+++ src/util.c
@@ -156,10 +156,17 @@
156 }
157 }
158 return zStart;
159 }
160
 
 
 
 
 
 
 
161 /*
162 ** Check the input string to ensure that it is safe to pass into system().
163 ** A string is unsafe for system() on unix if it contains any of the following:
164 **
165 ** * Any occurrance of '$' or '`' except after \
@@ -171,30 +178,30 @@
171 ** This routine is intended as a second line of defense against attack.
172 ** It should never fail. Dangerous shell strings should be detected and
173 ** fixed before calling fossil_system(). This routine serves only as a
174 ** safety net in case of bugs elsewhere in the system.
175 **
176 ** If an unsafe string is seen, the process aborts.
177 */
178 void fossil_assert_safe_command_string(const char *z){
179 int unsafe = 0;
180 #ifndef _WIN32
181 /* Unix */
182 int inQuote = 0;
183 int i, c;
184 for(i=0; (c = z[i])!=0; i++){
185 switch( c ){
186 case '$':
187 case '`': {
188 unsafe = i+1;
189 break;
190 }
191 case ';':
192 case '|':
193 case '&':
194 case '\n': {
195 if( inQuote==0 && z[i+1]!=0 ) unsafe = i+1;
196 break;
197 }
198 case '"':
199 case '\'': {
200 if( inQuote==0 ){
@@ -205,25 +212,52 @@
205 break;
206 }
207 case '\\': {
208 if( z[i+1]==0 ){
209 unsafe = i+1;
210 }else{
211 i++;
212 }
213 break;
214 }
215 }
216 }
 
217 #else
218 /* Windows */
219
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
220 #endif
221 if( unsafe ){
222 fossil_fatal("Unsafe command string: %s\n%*shere ----^",
223 z, unsafe+13, "");
 
 
 
 
 
224 }
 
225 }
226
227 /*
228 ** This function implements a cross-platform "system()" interface.
229 */
@@ -236,19 +270,20 @@
236 char *zNewCmd = mprintf("\"%s\"", zOrigCmd);
237 wchar_t *zUnicode = fossil_utf8_to_unicode(zNewCmd);
238 if( g.fSystemTrace ) {
239 fossil_trace("SYSTEM: %s\n", zNewCmd);
240 }
241 fossil_assert_safe_command_string(zOrigCmd);
242 rc = _wsystem(zUnicode);
 
243 fossil_unicode_free(zUnicode);
244 free(zNewCmd);
245 #else
246 /* On unix, evaluate the command directly.
247 */
248 if( g.fSystemTrace ) fprintf(stderr, "SYSTEM: %s\n", zOrigCmd);
249 fossil_assert_safe_command_string(zOrigCmd);
250
251 /* Unix systems should never shell-out while processing an HTTP request,
252 ** either via CGI, SCGI, or direct HTTP. The following assert verifies
253 ** this. And the following assert proves that Fossil is not vulnerable
254 ** to the ShellShock or BashDoor bug.
@@ -265,13 +300,16 @@
265
266 /*
267 ** COMMAND: test-fossil-system
268 **
269 ** Read lines of input and send them to fossil_system() for evaluation.
 
 
270 */
271 void test_fossil_system_cmd(void){
272 char zLine[10000];
 
273 while(1){
274 size_t n;
275 printf("system-test> ");
276 fflush(stdout);
277 if( !fgets(zLine, sizeof(zLine), stdin) ) break;
278
--- src/util.c
+++ src/util.c
@@ -156,10 +156,17 @@
156 }
157 }
158 return zStart;
159 }
160
161 /*
162 ** If this local variable is set, fossil_assert_safe_command_string()
163 ** returns false on an unsafe command-string rather than abort. Set
164 ** this variable for testing.
165 */
166 static int safeCmdStrTest = 0;
167
168 /*
169 ** Check the input string to ensure that it is safe to pass into system().
170 ** A string is unsafe for system() on unix if it contains any of the following:
171 **
172 ** * Any occurrance of '$' or '`' except after \
@@ -171,30 +178,30 @@
178 ** This routine is intended as a second line of defense against attack.
179 ** It should never fail. Dangerous shell strings should be detected and
180 ** fixed before calling fossil_system(). This routine serves only as a
181 ** safety net in case of bugs elsewhere in the system.
182 **
183 ** If an unsafe string is seen, either abort or return false.
184 */
185 static int fossil_assert_safe_command_string(const char *z){
186 int unsafe = 0;
187 #ifndef _WIN32
188 /* Unix */
189 int inQuote = 0;
190 int i, c;
191 for(i=0; !unsafe && (c = z[i])!=0; i++){
192 switch( c ){
193 case '$':
194 case '`': {
195 if( inQuote!='\'' ) unsafe = i+1;
196 break;
197 }
198 case ';':
199 case '|':
200 case '&':
201 case '\n': {
202 if( inQuote!='\'' && z[i+1]!=0 ) unsafe = i+1;
203 break;
204 }
205 case '"':
206 case '\'': {
207 if( inQuote==0 ){
@@ -205,25 +212,52 @@
212 break;
213 }
214 case '\\': {
215 if( z[i+1]==0 ){
216 unsafe = i+1;
217 }else if( inQuote!='\'' ){
218 i++;
219 }
220 break;
221 }
222 }
223 }
224 if( inQuote ) unsafe = i;
225 #else
226 /* Windows */
227 int i, c;
228 int inQuote = 0;
229 for(i=0; !unsafe && (c = z[i])!=0; i++){
230 switch( c ){
231 case '|':
232 case '&':
233 case '\n': {
234 if( inQuote==0 && z[i+1]!=0 ) unsafe = i+1;
235 break;
236 }
237 case '"': {
238 if( inQuote==c ){
239 inQuote = 0;
240 }else{
241 inQuote = c;
242 }
243 break;
244 }
245 }
246 }
247 if( inQuote ) unsafe = i;
248 #endif
249 if( unsafe ){
250 char *zMsg = mprintf("Unsafe command string: %s\n%*shere ----^",
251 z, unsafe+13, "");
252 if( safeCmdStrTest ){
253 fossil_print("%z\n", zMsg);
254 }else{
255 fossil_panic("%s", zMsg);
256 }
257 }
258 return !unsafe;
259 }
260
261 /*
262 ** This function implements a cross-platform "system()" interface.
263 */
@@ -236,19 +270,20 @@
270 char *zNewCmd = mprintf("\"%s\"", zOrigCmd);
271 wchar_t *zUnicode = fossil_utf8_to_unicode(zNewCmd);
272 if( g.fSystemTrace ) {
273 fossil_trace("SYSTEM: %s\n", zNewCmd);
274 }
275 if( fossil_assert_safe_command_string(zOrigCmd) ){
276 rc = _wsystem(zUnicode);
277 }
278 fossil_unicode_free(zUnicode);
279 free(zNewCmd);
280 #else
281 /* On unix, evaluate the command directly.
282 */
283 if( g.fSystemTrace ) fprintf(stderr, "SYSTEM: %s\n", zOrigCmd);
284 if( !fossil_assert_safe_command_string(zOrigCmd) ) return 1;
285
286 /* Unix systems should never shell-out while processing an HTTP request,
287 ** either via CGI, SCGI, or direct HTTP. The following assert verifies
288 ** this. And the following assert proves that Fossil is not vulnerable
289 ** to the ShellShock or BashDoor bug.
@@ -265,13 +300,16 @@
300
301 /*
302 ** COMMAND: test-fossil-system
303 **
304 ** Read lines of input and send them to fossil_system() for evaluation.
305 ** Use this command to verify that fossil_system() will not run "unsafe"
306 ** commands.
307 */
308 void test_fossil_system_cmd(void){
309 char zLine[10000];
310 safeCmdStrTest = 1;
311 while(1){
312 size_t n;
313 printf("system-test> ");
314 fflush(stdout);
315 if( !fgets(zLine, sizeof(zLine), stdin) ) break;
316

Keyboard Shortcuts

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