Fossil

Check-in [3b191c984b]
Login

Many hyperlinks are disabled.
Use anonymous login to enable hyperlinks.

Overview
Comment: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.
Downloads: Tarball | ZIP archive
Timelines: family | ancestors | descendants | both | trunk
Files: files | file ages | folders
SHA3-256: 3b191c984b831571c6a67aa9ce862dac6717a06e98797ab409a82bfe2b3390d6
User & Date: drh 2017-08-12 18:15:36.005
References
2017-08-23
17:28
Fix build (previous cherry-pick was not complete) (cherry-pick): For temporary filename paths on Windows, changes all backslash characters into forward slashes, so that the new enhanced-security shell escape mechanism from check-in [3b191c98] can use those temporary filenames. check-in: dbda6e2a5d user: jan.nijtmans tags: branch-2.3
17:18
For temporary filename paths on Windows, changes all backslash characters into forward slashes, so that the new enhanced-security shell escape mechanism from check-in [3b191c98] can use those temporary filenames. check-in: e474c177df user: drh tags: trunk
2017-08-12
16:20
Avoid another attack vector when using SSH sync protocol by not calling a shell interpreter. Fixes only Unix-like environments by using execvp() instead of a string that can be mishandled by /bin/sh. Superseded by [3b191c984b] &co. Closed-Leaf check-in: ce7baa9798 user: andybradford tags: ssh-shell-cleanup
Context
2017-08-12
18:20
Fix the needEscape calculation in blob_append_escaped_arg(). check-in: 9690d370e0 user: drh tags: trunk
18:15
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. check-in: 3b191c984b user: drh tags: trunk
04:19
Typo correction check-in: 45a3d4b167 user: andygoth tags: trunk
Changes
Unified Diff Ignore Whitespace Patch
Changes to src/blob.c.
1167
1168
1169
1170
1171
1172
1173

1174










1175
1176



1177
1178
1179




1180
1181
1182

1183


1184
1185

1186
1187
1188
1189
1190





1191

1192
1193
1194
1195
1196
1197
1198
    }else{
      z[--j] = z[i];
    }
  }
}

/*

** Shell-escape the given string.  Append the result to a blob.










*/
void shell_escape(Blob *pBlob, const char *zIn){



  int n = blob_size(pBlob);
  int k = strlen(zIn);
  int i, c;




  char *z;
  for(i=0; (c = zIn[i])!=0; i++){
    if( fossil_isspace(c) || c=='"' || (c=='\\' && zIn[i+1]!=0) ){

      blob_appendf(pBlob, "\"%s\"", zIn);


      z = blob_buffer(pBlob);
      for(i=n+1; i<=n+k; i++){

        if( z[i]=='"' ) z[i] = '_';
      }
      return;
    }
  }





  blob_append(pBlob, zIn, -1);

}

/*
** A read(2)-like impl for the Blob class. Reads (copies) up to nLen
** bytes from pIn, starting at position pIn->iCursor, and copies them
** to pDest (which must be valid memory at least nLen bytes long).
**







>
|
>
>
>
>
>
>
>
>
>
>

|
>
>
>

|
|
>
>
>
>
|

|
>
|
>
>
|
<
>
|

<


>
>
>
>
>

>







1167
1168
1169
1170
1171
1172
1173
1174
1175
1176
1177
1178
1179
1180
1181
1182
1183
1184
1185
1186
1187
1188
1189
1190
1191
1192
1193
1194
1195
1196
1197
1198
1199
1200
1201
1202
1203
1204
1205

1206
1207
1208

1209
1210
1211
1212
1213
1214
1215
1216
1217
1218
1219
1220
1221
1222
1223
1224
    }else{
      z[--j] = z[i];
    }
  }
}

/*
** pBlob is a shell command under construction.  This routine safely
** appends argument zIn.
**
** The argument is escaped if it contains white space or other characters
** that need to be escaped for the shell.  If zIn contains characters
** that cannot be safely escaped, then throw a fatal error.
**
** The argument is expected to a filename of some kinds.  As shell commands
** commonly have command-line options that begin with "-" and since we
** do not want an attacker to be able to invoke these switches using
** filenames that begin with "-", if zIn begins with "-", prepend
** an additional "./".
*/
void blob_append_escaped_arg(Blob *pBlob, const char *zIn){
  int i;
  char c;
  int needEscape = 0;
  int n = blob_size(pBlob);
  char *z = blob_buffer(pBlob);
#if defined(_WIN32_)
  const char cQuote = '"';    /* Use "..." quoting on windows */
#else
  const char cQuote = '\'';   /* Use '...' quoting on unix */
#endif

  for(i=0; (c = zIn[i])!=0; i++){
    if( c==cQuote || c=='\\' || c<' ' ) {
      Blob bad;
      blob_token(pBlob, &bad);
      fossil_fatal("the [%s] argument to the \"%s\" command contains "
                   "a character (ascii 0x%02x) that is a security risk",
                   zIn, blob_str(&bad), c);

      if( !needEscape && !fossil_isspace(c) && c!='/' && c!='.' && c!='_' ){
        needEscape = 1;
      }

    }
  }
  if( n>0 && !fossil_isspace(z[n-1]) ){
    blob_append(pBlob, " ", 1);
  }
  if( needEscape ) blob_append(pBlob, &cQuote, 1);
  if( zIn[0]=='-' ) blob_append(pBlob, "./", 2);
  blob_append(pBlob, zIn, -1);
  if( needEscape ) blob_append(pBlob, &cQuote, 1);
}

/*
** A read(2)-like impl for the Blob class. Reads (copies) up to nLen
** bytes from pIn, starting at position pIn->iCursor, and copies them
** to pDest (which must be valid memory at least nLen bytes long).
**
Changes to src/diffcmd.c.
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
      blob_reset(&nameFile1);
      blob_appendf(&nameFile1, "%s~%d", zFile2, cnt++);
    }while( file_access(blob_str(&nameFile1),F_OK)==0 );
    blob_write_to_file(pFile1, blob_str(&nameFile1));

    /* Construct the external diff command */
    blob_zero(&cmd);
    blob_appendf(&cmd, "%s ", zDiffCmd);
    if( fSwapDiff ){
      shell_escape(&cmd, zFile2);
      blob_append(&cmd, " ", 1);
      shell_escape(&cmd, blob_str(&nameFile1));
    }else{
      shell_escape(&cmd, blob_str(&nameFile1));
      blob_append(&cmd, " ", 1);
      shell_escape(&cmd, zFile2);
    }

    /* Run the external diff command */
    fossil_system(blob_str(&cmd));

    /* Delete the temporary file and clean up memory used */
    file_delete(blob_str(&nameFile1));







|

|
<
|

|
<
|







261
262
263
264
265
266
267
268
269
270

271
272
273

274
275
276
277
278
279
280
281
      blob_reset(&nameFile1);
      blob_appendf(&nameFile1, "%s~%d", zFile2, cnt++);
    }while( file_access(blob_str(&nameFile1),F_OK)==0 );
    blob_write_to_file(pFile1, blob_str(&nameFile1));

    /* Construct the external diff command */
    blob_zero(&cmd);
    blob_append(&cmd, zDiffCmd, -1);
    if( fSwapDiff ){
      blob_append_escaped_arg(&cmd, zFile2);

      blob_append_escaped_arg(&cmd, blob_str(&nameFile1));
    }else{
      blob_append_escaped_arg(&cmd, blob_str(&nameFile1));

      blob_append_escaped_arg(&cmd, zFile2);
    }

    /* Run the external diff command */
    fossil_system(blob_str(&cmd));

    /* Delete the temporary file and clean up memory used */
    file_delete(blob_str(&nameFile1));
358
359
360
361
362
363
364
365
366
367

368
369
370
371
372
373
374
375
    file_tempname(&temp1, blob_str(&prefix1));
    file_tempname(&temp2, blob_str(&prefix2));
    blob_write_to_file(pFile1, blob_str(&temp1));
    blob_write_to_file(pFile2, blob_str(&temp2));

    /* Construct the external diff command */
    blob_zero(&cmd);
    blob_appendf(&cmd, "%s ", zDiffCmd);
    shell_escape(&cmd, blob_str(&temp1));
    blob_append(&cmd, " ", 1);

    shell_escape(&cmd, blob_str(&temp2));

    /* Run the external diff command */
    fossil_system(blob_str(&cmd));

    /* Delete the temporary file and clean up memory used */
    file_delete(blob_str(&temp1));
    file_delete(blob_str(&temp2));







<
<
|
>
|







356
357
358
359
360
361
362


363
364
365
366
367
368
369
370
371
372
    file_tempname(&temp1, blob_str(&prefix1));
    file_tempname(&temp2, blob_str(&prefix2));
    blob_write_to_file(pFile1, blob_str(&temp1));
    blob_write_to_file(pFile2, blob_str(&temp2));

    /* Construct the external diff command */
    blob_zero(&cmd);


    blob_append(&cmd, zDiffCmd, -1);
    blob_append_escaped_arg(&cmd, blob_str(&temp1));
    blob_append_escaped_arg(&cmd, blob_str(&temp2));

    /* Run the external diff command */
    fossil_system(blob_str(&cmd));

    /* Delete the temporary file and clean up memory used */
    file_delete(blob_str(&temp1));
    file_delete(blob_str(&temp2));
Changes to src/http_transport.c.
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128


129
130
131
132
133
134
135
136
137
138
139
140


141
142
143
144
145
146
147
148
149
150
151
152
  if( pnRcvd ) *pnRcvd = transport.nRcvd;
  if( resetFlag ){
    transport.nSent = 0;
    transport.nRcvd = 0;
  }
}

/*
** Remove leading "-" characters from the input string.
**
** This prevents attacks that try to trick a victim into using
** a ssh:// URI with a carefully crafted hostname of other
** parameter that ends up being interpreted as a command-line
** option by "ssh".
*/
static const char *stripLeadingMinus(const char *z){
  while( z[0]=='-' ) z++;
  return z;
}

/*
** Default SSH command
*/
#ifdef _WIN32
static const char zDefaultSshCmd[] = "plink -ssh -T";
#else
static const char zDefaultSshCmd[] = "ssh -e none -T";
#endif

/*
** SSH initialization of the transport layer
*/
int transport_ssh_open(UrlData *pUrlData){
  /* For SSH we need to create and run SSH fossil http
  ** to talk to the remote machine.
  */
  char *zSsh;        /* The base SSH command */
  Blob zCmd;         /* The SSH command */
  char *zHost;       /* The host name to contact */
  int n;             /* Size of prefix string */

  socket_ssh_resolve_addr(pUrlData);
  zSsh = db_get("ssh-command", zDefaultSshCmd);
  blob_init(&zCmd, zSsh, -1);
  if( pUrlData->port!=pUrlData->dfltPort && pUrlData->port ){
#ifdef _WIN32
    blob_appendf(&zCmd, " -P %d", pUrlData->port);
#else
    blob_appendf(&zCmd, " -p %d", pUrlData->port);
#endif
  }
  if( g.fSshTrace ){
    fossil_force_newline();
    fossil_print("%s", blob_str(&zCmd));  /* Show the base of the SSH command */
  }
  if( pUrlData->user && pUrlData->user[0] ){
    zHost = mprintf("%s@%s", pUrlData->user, pUrlData->name);


  }else{
    zHost = mprintf("%s", pUrlData->name);
  }
  n = blob_size(&zCmd);
  blob_append(&zCmd, " ", 1);
  shell_escape(&zCmd, stripLeadingMinus(zHost));
  blob_append(&zCmd, " ", 1);
  shell_escape(&zCmd, mprintf("%s", pUrlData->fossil));
  blob_append(&zCmd, " test-http", 10);
  if( pUrlData->path && pUrlData->path[0] ){
    blob_append(&zCmd, " ", 1);
    shell_escape(&zCmd, mprintf("%s", stripLeadingMinus(pUrlData->path)));


  }
  if( g.fSshTrace ){
    fossil_print("%s\n", blob_str(&zCmd)+n);  /* Show tail of SSH command */
  }
  free(zHost);
  popen2(blob_str(&zCmd), &sshIn, &sshOut, &sshPid);
  if( sshPid==0 ){
    socket_set_errmsg("cannot start ssh tunnel using [%b]", &zCmd);
  }
  blob_reset(&zCmd);
  return sshPid==0;
}







<
<
<
<
<
<
<
<
<
<
<
<
<



















<











<
<
<
<


>
>

|

<
<
<
<
|


|
<
>
>


|

<







72
73
74
75
76
77
78













79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97

98
99
100
101
102
103
104
105
106
107
108




109
110
111
112
113
114
115




116
117
118
119

120
121
122
123
124
125

126
127
128
129
130
131
132
  if( pnRcvd ) *pnRcvd = transport.nRcvd;
  if( resetFlag ){
    transport.nSent = 0;
    transport.nRcvd = 0;
  }
}














/*
** Default SSH command
*/
#ifdef _WIN32
static const char zDefaultSshCmd[] = "plink -ssh -T";
#else
static const char zDefaultSshCmd[] = "ssh -e none -T";
#endif

/*
** SSH initialization of the transport layer
*/
int transport_ssh_open(UrlData *pUrlData){
  /* For SSH we need to create and run SSH fossil http
  ** to talk to the remote machine.
  */
  char *zSsh;        /* The base SSH command */
  Blob zCmd;         /* The SSH command */
  char *zHost;       /* The host name to contact */


  socket_ssh_resolve_addr(pUrlData);
  zSsh = db_get("ssh-command", zDefaultSshCmd);
  blob_init(&zCmd, zSsh, -1);
  if( pUrlData->port!=pUrlData->dfltPort && pUrlData->port ){
#ifdef _WIN32
    blob_appendf(&zCmd, " -P %d", pUrlData->port);
#else
    blob_appendf(&zCmd, " -p %d", pUrlData->port);
#endif
  }




  if( pUrlData->user && pUrlData->user[0] ){
    zHost = mprintf("%s@%s", pUrlData->user, pUrlData->name);
    blob_append_escaped_arg(&zCmd, zHost);
    fossil_free(zHost);
  }else{
    blob_append_escaped_arg(&zCmd, pUrlData->name);
  }




  blob_append_escaped_arg(&zCmd, pUrlData->fossil);
  blob_append(&zCmd, " test-http", 10);
  if( pUrlData->path && pUrlData->path[0] ){
    blob_append_escaped_arg(&zCmd, pUrlData->path);

  }else{
    fossil_fatal("ssh:// URI does not specify a path to the repository");
  }
  if( g.fSshTrace ){
    fossil_print("%s\n", blob_str(&zCmd));  /* Show the whole SSH command */
  }

  popen2(blob_str(&zCmd), &sshIn, &sshOut, &sshPid);
  if( sshPid==0 ){
    socket_set_errmsg("cannot start ssh tunnel using [%b]", &zCmd);
  }
  blob_reset(&zCmd);
  return sshPid==0;
}