Fossil SCM

Change the shell_escape() procedure into blob_append_escaped_arg(). Have that procedure raise a fatal error if the argument to be appended contains dodgy characters that might pose a security risk. Also, prepend "./" in front of arguments that begin with "-" to prevent them from looking like switches.

drh 2017-08-12 18:15 trunk
Commit 3b191c984b831571c6a67aa9ce862dac6717a06e98797ab409a82bfe2b3390d6
Parent 45a3d4b1670f123…
3 files changed +44 -18 +8 -11 +8 -28
~ src/blob.c ~ src/diffcmd.c ~ src/http_transport.c
M src/blob.c
+44 -18
--- src/blob.c
+++ src/blob.c
@@ -1169,28 +1169,54 @@
11691169
 }
11701170
 }
11711171
 }
11721172
11731173
 /*
1174
-** Shell-escape the given string. Append the result to a blob.
1175
-*/
1176
-void shell_escape(Blob *pBlob, const char *zIn){
1177
- int n = blob_size(pBlob);
1178
- int k = strlen(zIn);
1179
- int i, c;
1180
- char *z;
1181
- for(i=0; (c = zIn[i])!=0; i++){
1182
- if( fossil_isspace(c) || c=='"' || (c=='\\' && zIn[i+1]!=0) ){
1183
- blob_appendf(pBlob, "\"%s\"", zIn);
1184
- z = blob_buffer(pBlob);
1185
- for(i=n+1; i<=n+k; i++){
1186
- if( z[i]=='"' ) z[i] = '_';
1187
- }
1188
- return;
1189
- }
1190
- }
1191
- blob_append(pBlob, zIn, -1);
1174
+** pBlob is a shell command under construction. This routine safely
1175
+** appends argument zIn.
1176
+**
1177
+** The argument is escaped if it contains white space or other characters
1178
+** that need to be escaped for the shell. If zIn contains characters
1179
+** that cannot be safely escaped, then throw a fatal error.
1180
+**
1181
+** The argument is expected to a filename of some kinds. As shell commands
1182
+** commonly have command-line options that begin with "-" and since we
1183
+** do not want an attacker to be able to invoke these switches using
1184
+** filenames that begin with "-", if zIn begins with "-", prepend
1185
+** an additional "./".
1186
+*/
1187
+void blob_append_escaped_arg(Blob *pBlob, const char *zIn){
1188
+ int i;
1189
+ char c;
1190
+ int needEscape = 0;
1191
+ int n = blob_size(pBlob);
1192
+ char *z = blob_buffer(pBlob);
1193
+#if defined(_WIN32_)
1194
+ const char cQuote = '"'; /* Use "..." quoting on windows */
1195
+#else
1196
+ const char cQuote = '\''; /* Use '...' quoting on unix */
1197
+#endif
1198
+
1199
+ for(i=0; (c = zIn[i])!=0; i++){
1200
+ if( c==cQuote || c=='\\' || c<' ' ) {
1201
+ Blob bad;
1202
+ blob_token(pBlob, &bad);
1203
+ fossil_fatal("the [%s] argument to the \"%s\" command contains "
1204
+ "a character (ascii 0x%02x) that is a security risk",
1205
+ zIn, blob_str(&bad), c);
1206
+ if( !needEscape && !fossil_isspace(c) && c!='/' && c!='.' && c!='_' ){
1207
+ needEscape = 1;
1208
+ }
1209
+ }
1210
+ }
1211
+ if( n>0 && !fossil_isspace(z[n-1]) ){
1212
+ blob_append(pBlob, " ", 1);
1213
+ }
1214
+ if( needEscape ) blob_append(pBlob, &cQuote, 1);
1215
+ if( zIn[0]=='-' ) blob_append(pBlob, "./", 2);
1216
+ blob_append(pBlob, zIn, -1);
1217
+ if( needEscape ) blob_append(pBlob, &cQuote, 1);
11921218
 }
11931219
11941220
 /*
11951221
 ** A read(2)-like impl for the Blob class. Reads (copies) up to nLen
11961222
 ** bytes from pIn, starting at position pIn->iCursor, and copies them
11971223
--- src/blob.c
+++ src/blob.c
@@ -1169,28 +1169,54 @@
1169 }
1170 }
1171 }
1172
1173 /*
1174 ** Shell-escape the given string. Append the result to a blob.
1175 */
1176 void shell_escape(Blob *pBlob, const char *zIn){
1177 int n = blob_size(pBlob);
1178 int k = strlen(zIn);
1179 int i, c;
1180 char *z;
1181 for(i=0; (c = zIn[i])!=0; i++){
1182 if( fossil_isspace(c) || c=='"' || (c=='\\' && zIn[i+1]!=0) ){
1183 blob_appendf(pBlob, "\"%s\"", zIn);
1184 z = blob_buffer(pBlob);
1185 for(i=n+1; i<=n+k; i++){
1186 if( z[i]=='"' ) z[i] = '_';
1187 }
1188 return;
1189 }
1190 }
1191 blob_append(pBlob, zIn, -1);
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1192 }
1193
1194 /*
1195 ** A read(2)-like impl for the Blob class. Reads (copies) up to nLen
1196 ** bytes from pIn, starting at position pIn->iCursor, and copies them
1197
--- src/blob.c
+++ src/blob.c
@@ -1169,28 +1169,54 @@
1169 }
1170 }
1171 }
1172
1173 /*
1174 ** pBlob is a shell command under construction. This routine safely
1175 ** appends argument zIn.
1176 **
1177 ** The argument is escaped if it contains white space or other characters
1178 ** that need to be escaped for the shell. If zIn contains characters
1179 ** that cannot be safely escaped, then throw a fatal error.
1180 **
1181 ** The argument is expected to a filename of some kinds. As shell commands
1182 ** commonly have command-line options that begin with "-" and since we
1183 ** do not want an attacker to be able to invoke these switches using
1184 ** filenames that begin with "-", if zIn begins with "-", prepend
1185 ** an additional "./".
1186 */
1187 void blob_append_escaped_arg(Blob *pBlob, const char *zIn){
1188 int i;
1189 char c;
1190 int needEscape = 0;
1191 int n = blob_size(pBlob);
1192 char *z = blob_buffer(pBlob);
1193 #if defined(_WIN32_)
1194 const char cQuote = '"'; /* Use "..." quoting on windows */
1195 #else
1196 const char cQuote = '\''; /* Use '...' quoting on unix */
1197 #endif
1198
1199 for(i=0; (c = zIn[i])!=0; i++){
1200 if( c==cQuote || c=='\\' || c<' ' ) {
1201 Blob bad;
1202 blob_token(pBlob, &bad);
1203 fossil_fatal("the [%s] argument to the \"%s\" command contains "
1204 "a character (ascii 0x%02x) that is a security risk",
1205 zIn, blob_str(&bad), c);
1206 if( !needEscape && !fossil_isspace(c) && c!='/' && c!='.' && c!='_' ){
1207 needEscape = 1;
1208 }
1209 }
1210 }
1211 if( n>0 && !fossil_isspace(z[n-1]) ){
1212 blob_append(pBlob, " ", 1);
1213 }
1214 if( needEscape ) blob_append(pBlob, &cQuote, 1);
1215 if( zIn[0]=='-' ) blob_append(pBlob, "./", 2);
1216 blob_append(pBlob, zIn, -1);
1217 if( needEscape ) blob_append(pBlob, &cQuote, 1);
1218 }
1219
1220 /*
1221 ** A read(2)-like impl for the Blob class. Reads (copies) up to nLen
1222 ** bytes from pIn, starting at position pIn->iCursor, and copies them
1223
M src/diffcmd.c
+8 -11
--- src/diffcmd.c
+++ src/diffcmd.c
@@ -263,19 +263,17 @@
263263
 }while( file_access(blob_str(&nameFile1),F_OK)==0 );
264264
 blob_write_to_file(pFile1, blob_str(&nameFile1));
265265
266266
 /* Construct the external diff command */
267267
 blob_zero(&cmd);
268
- blob_appendf(&cmd, "%s ", zDiffCmd);
268
+ blob_append(&cmd, zDiffCmd, -1);
269269
 if( fSwapDiff ){
270
- shell_escape(&cmd, zFile2);
271
- blob_append(&cmd, " ", 1);
272
- shell_escape(&cmd, blob_str(&nameFile1));
270
+ blob_append_escaped_arg(&cmd, zFile2);
271
+ blob_append_escaped_arg(&cmd, blob_str(&nameFile1));
273272
 }else{
274
- shell_escape(&cmd, blob_str(&nameFile1));
275
- blob_append(&cmd, " ", 1);
276
- shell_escape(&cmd, zFile2);
273
+ blob_append_escaped_arg(&cmd, blob_str(&nameFile1));
274
+ blob_append_escaped_arg(&cmd, zFile2);
277275
 }
278276
279277
 /* Run the external diff command */
280278
 fossil_system(blob_str(&cmd));
281279
@@ -360,14 +358,13 @@
360358
 blob_write_to_file(pFile1, blob_str(&temp1));
361359
 blob_write_to_file(pFile2, blob_str(&temp2));
362360
363361
 /* Construct the external diff command */
364362
 blob_zero(&cmd);
365
- blob_appendf(&cmd, "%s ", zDiffCmd);
366
- shell_escape(&cmd, blob_str(&temp1));
367
- blob_append(&cmd, " ", 1);
368
- shell_escape(&cmd, blob_str(&temp2));
363
+ blob_append(&cmd, zDiffCmd, -1);
364
+ blob_append_escaped_arg(&cmd, blob_str(&temp1));
365
+ blob_append_escaped_arg(&cmd, blob_str(&temp2));
369366
370367
 /* Run the external diff command */
371368
 fossil_system(blob_str(&cmd));
372369
373370
 /* Delete the temporary file and clean up memory used */
374371
--- src/diffcmd.c
+++ src/diffcmd.c
@@ -263,19 +263,17 @@
263 }while( file_access(blob_str(&nameFile1),F_OK)==0 );
264 blob_write_to_file(pFile1, blob_str(&nameFile1));
265
266 /* Construct the external diff command */
267 blob_zero(&cmd);
268 blob_appendf(&cmd, "%s ", zDiffCmd);
269 if( fSwapDiff ){
270 shell_escape(&cmd, zFile2);
271 blob_append(&cmd, " ", 1);
272 shell_escape(&cmd, blob_str(&nameFile1));
273 }else{
274 shell_escape(&cmd, blob_str(&nameFile1));
275 blob_append(&cmd, " ", 1);
276 shell_escape(&cmd, zFile2);
277 }
278
279 /* Run the external diff command */
280 fossil_system(blob_str(&cmd));
281
@@ -360,14 +358,13 @@
360 blob_write_to_file(pFile1, blob_str(&temp1));
361 blob_write_to_file(pFile2, blob_str(&temp2));
362
363 /* Construct the external diff command */
364 blob_zero(&cmd);
365 blob_appendf(&cmd, "%s ", zDiffCmd);
366 shell_escape(&cmd, blob_str(&temp1));
367 blob_append(&cmd, " ", 1);
368 shell_escape(&cmd, blob_str(&temp2));
369
370 /* Run the external diff command */
371 fossil_system(blob_str(&cmd));
372
373 /* Delete the temporary file and clean up memory used */
374
--- src/diffcmd.c
+++ src/diffcmd.c
@@ -263,19 +263,17 @@
263 }while( file_access(blob_str(&nameFile1),F_OK)==0 );
264 blob_write_to_file(pFile1, blob_str(&nameFile1));
265
266 /* Construct the external diff command */
267 blob_zero(&cmd);
268 blob_append(&cmd, zDiffCmd, -1);
269 if( fSwapDiff ){
270 blob_append_escaped_arg(&cmd, zFile2);
271 blob_append_escaped_arg(&cmd, blob_str(&nameFile1));
 
272 }else{
273 blob_append_escaped_arg(&cmd, blob_str(&nameFile1));
274 blob_append_escaped_arg(&cmd, zFile2);
 
275 }
276
277 /* Run the external diff command */
278 fossil_system(blob_str(&cmd));
279
@@ -360,14 +358,13 @@
358 blob_write_to_file(pFile1, blob_str(&temp1));
359 blob_write_to_file(pFile2, blob_str(&temp2));
360
361 /* Construct the external diff command */
362 blob_zero(&cmd);
363 blob_append(&cmd, zDiffCmd, -1);
364 blob_append_escaped_arg(&cmd, blob_str(&temp1));
365 blob_append_escaped_arg(&cmd, blob_str(&temp2));
 
366
367 /* Run the external diff command */
368 fossil_system(blob_str(&cmd));
369
370 /* Delete the temporary file and clean up memory used */
371
M src/http_transport.c
+8 -28
--- src/http_transport.c
+++ src/http_transport.c
@@ -74,23 +74,10 @@
7474
 transport.nSent = 0;
7575
 transport.nRcvd = 0;
7676
 }
7777
 }
7878
79
-/*
80
-** Remove leading "-" characters from the input string.
81
-**
82
-** This prevents attacks that try to trick a victim into using
83
-** a ssh:// URI with a carefully crafted hostname of other
84
-** parameter that ends up being interpreted as a command-line
85
-** option by "ssh".
86
-*/
87
-static const char *stripLeadingMinus(const char *z){
88
- while( z[0]=='-' ) z++;
89
- return z;
90
-}
91
-
9279
 /*
9380
 ** Default SSH command
9481
 */
9582
 #ifdef _WIN32
9683
 static const char zDefaultSshCmd[] = "plink -ssh -T";
@@ -106,11 +93,10 @@
10693
 ** to talk to the remote machine.
10794
 */
10895
 char *zSsh; /* The base SSH command */
10996
 Blob zCmd; /* The SSH command */
11097
 char *zHost; /* The host name to contact */
111
- int n; /* Size of prefix string */
11298
11399
 socket_ssh_resolve_addr(pUrlData);
114100
 zSsh = db_get("ssh-command", zDefaultSshCmd);
115101
 blob_init(&zCmd, zSsh, -1);
116102
 if( pUrlData->port!=pUrlData->dfltPort && pUrlData->port ){
@@ -118,33 +104,27 @@
118104
 blob_appendf(&zCmd, " -P %d", pUrlData->port);
119105
 #else
120106
 blob_appendf(&zCmd, " -p %d", pUrlData->port);
121107
 #endif
122108
 }
123
- if( g.fSshTrace ){
124
- fossil_force_newline();
125
- fossil_print("%s", blob_str(&zCmd)); /* Show the base of the SSH command */
126
- }
127109
 if( pUrlData->user && pUrlData->user[0] ){
128110
 zHost = mprintf("%s@%s", pUrlData->user, pUrlData->name);
111
+ blob_append_escaped_arg(&zCmd, zHost);
112
+ fossil_free(zHost);
129113
 }else{
130
- zHost = mprintf("%s", pUrlData->name);
114
+ blob_append_escaped_arg(&zCmd, pUrlData->name);
131115
 }
132
- n = blob_size(&zCmd);
133
- blob_append(&zCmd, " ", 1);
134
- shell_escape(&zCmd, stripLeadingMinus(zHost));
135
- blob_append(&zCmd, " ", 1);
136
- shell_escape(&zCmd, mprintf("%s", pUrlData->fossil));
116
+ blob_append_escaped_arg(&zCmd, pUrlData->fossil);
137117
 blob_append(&zCmd, " test-http", 10);
138118
 if( pUrlData->path && pUrlData->path[0] ){
139
- blob_append(&zCmd, " ", 1);
140
- shell_escape(&zCmd, mprintf("%s", stripLeadingMinus(pUrlData->path)));
119
+ blob_append_escaped_arg(&zCmd, pUrlData->path);
120
+ }else{
121
+ fossil_fatal("ssh:// URI does not specify a path to the repository");
141122
 }
142123
 if( g.fSshTrace ){
143
- fossil_print("%s\n", blob_str(&zCmd)+n); /* Show tail of SSH command */
124
+ fossil_print("%s\n", blob_str(&zCmd)); /* Show the whole SSH command */
144125
 }
145
- free(zHost);
146126
 popen2(blob_str(&zCmd), &sshIn, &sshOut, &sshPid);
147127
 if( sshPid==0 ){
148128
 socket_set_errmsg("cannot start ssh tunnel using [%b]", &zCmd);
149129
 }
150130
 blob_reset(&zCmd);
151131
--- src/http_transport.c
+++ src/http_transport.c
@@ -74,23 +74,10 @@
74 transport.nSent = 0;
75 transport.nRcvd = 0;
76 }
77 }
78
79 /*
80 ** Remove leading "-" characters from the input string.
81 **
82 ** This prevents attacks that try to trick a victim into using
83 ** a ssh:// URI with a carefully crafted hostname of other
84 ** parameter that ends up being interpreted as a command-line
85 ** option by "ssh".
86 */
87 static const char *stripLeadingMinus(const char *z){
88 while( z[0]=='-' ) z++;
89 return z;
90 }
91
92 /*
93 ** Default SSH command
94 */
95 #ifdef _WIN32
96 static const char zDefaultSshCmd[] = "plink -ssh -T";
@@ -106,11 +93,10 @@
106 ** to talk to the remote machine.
107 */
108 char *zSsh; /* The base SSH command */
109 Blob zCmd; /* The SSH command */
110 char *zHost; /* The host name to contact */
111 int n; /* Size of prefix string */
112
113 socket_ssh_resolve_addr(pUrlData);
114 zSsh = db_get("ssh-command", zDefaultSshCmd);
115 blob_init(&zCmd, zSsh, -1);
116 if( pUrlData->port!=pUrlData->dfltPort && pUrlData->port ){
@@ -118,33 +104,27 @@
118 blob_appendf(&zCmd, " -P %d", pUrlData->port);
119 #else
120 blob_appendf(&zCmd, " -p %d", pUrlData->port);
121 #endif
122 }
123 if( g.fSshTrace ){
124 fossil_force_newline();
125 fossil_print("%s", blob_str(&zCmd)); /* Show the base of the SSH command */
126 }
127 if( pUrlData->user && pUrlData->user[0] ){
128 zHost = mprintf("%s@%s", pUrlData->user, pUrlData->name);
 
 
129 }else{
130 zHost = mprintf("%s", pUrlData->name);
131 }
132 n = blob_size(&zCmd);
133 blob_append(&zCmd, " ", 1);
134 shell_escape(&zCmd, stripLeadingMinus(zHost));
135 blob_append(&zCmd, " ", 1);
136 shell_escape(&zCmd, mprintf("%s", pUrlData->fossil));
137 blob_append(&zCmd, " test-http", 10);
138 if( pUrlData->path && pUrlData->path[0] ){
139 blob_append(&zCmd, " ", 1);
140 shell_escape(&zCmd, mprintf("%s", stripLeadingMinus(pUrlData->path)));
 
141 }
142 if( g.fSshTrace ){
143 fossil_print("%s\n", blob_str(&zCmd)+n); /* Show tail of SSH command */
144 }
145 free(zHost);
146 popen2(blob_str(&zCmd), &sshIn, &sshOut, &sshPid);
147 if( sshPid==0 ){
148 socket_set_errmsg("cannot start ssh tunnel using [%b]", &zCmd);
149 }
150 blob_reset(&zCmd);
151
--- src/http_transport.c
+++ src/http_transport.c
@@ -74,23 +74,10 @@
74 transport.nSent = 0;
75 transport.nRcvd = 0;
76 }
77 }
78
 
 
 
 
 
 
 
 
 
 
 
 
 
79 /*
80 ** Default SSH command
81 */
82 #ifdef _WIN32
83 static const char zDefaultSshCmd[] = "plink -ssh -T";
@@ -106,11 +93,10 @@
93 ** to talk to the remote machine.
94 */
95 char *zSsh; /* The base SSH command */
96 Blob zCmd; /* The SSH command */
97 char *zHost; /* The host name to contact */
 
98
99 socket_ssh_resolve_addr(pUrlData);
100 zSsh = db_get("ssh-command", zDefaultSshCmd);
101 blob_init(&zCmd, zSsh, -1);
102 if( pUrlData->port!=pUrlData->dfltPort && pUrlData->port ){
@@ -118,33 +104,27 @@
104 blob_appendf(&zCmd, " -P %d", pUrlData->port);
105 #else
106 blob_appendf(&zCmd, " -p %d", pUrlData->port);
107 #endif
108 }
 
 
 
 
109 if( pUrlData->user && pUrlData->user[0] ){
110 zHost = mprintf("%s@%s", pUrlData->user, pUrlData->name);
111 blob_append_escaped_arg(&zCmd, zHost);
112 fossil_free(zHost);
113 }else{
114 blob_append_escaped_arg(&zCmd, pUrlData->name);
115 }
116 blob_append_escaped_arg(&zCmd, pUrlData->fossil);
 
 
 
 
117 blob_append(&zCmd, " test-http", 10);
118 if( pUrlData->path && pUrlData->path[0] ){
119 blob_append_escaped_arg(&zCmd, pUrlData->path);
120 }else{
121 fossil_fatal("ssh:// URI does not specify a path to the repository");
122 }
123 if( g.fSshTrace ){
124 fossil_print("%s\n", blob_str(&zCmd)); /* Show the whole SSH command */
125 }
 
126 popen2(blob_str(&zCmd), &sshIn, &sshOut, &sshPid);
127 if( sshPid==0 ){
128 socket_set_errmsg("cannot start ssh tunnel using [%b]", &zCmd);
129 }
130 blob_reset(&zCmd);
131

Keyboard Shortcuts

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