Fossil

Check-in [4d13d948]
Login

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

Overview
Comment:Merge codecheck1 enhancements from trunk.
Downloads: Tarball | ZIP archive | SQL archive
Timelines: family | ancestors | descendants | both | email-alerts
Files: files | file ages | folders
SHA3-256:4d13d94893b5b7a7132d237dc5b90a570813e085952762625e5045542ce5d1c0
User & Date: drh 2018-06-21 17:07:18
Context
2018-06-21
19:10
The /subscribe page now creates entries in the subscriber table and sends verification emails. check-in: 31be2e17 user: drh tags: email-alerts
17:07
Merge codecheck1 enhancements from trunk. check-in: 4d13d948 user: drh tags: email-alerts
16:40
Strengthen the codecheck1.c utility program to help find cases where query parameters are used in unsafe ways. No unsafe usage of query parameters was detected in the current code. check-in: bb9233a6 user: drh tags: trunk
15:19
Merge the popen() on windows fix from trunk. check-in: ef2426dc user: drh tags: email-alerts
Changes
Hide Diffs Unified Diffs Ignore Whitespace Patch

Changes to src/codecheck1.c.

37
38
39
40
41
42
43





44
45
46
47
48
49
50
...
196
197
198
199
200
201
202






















203
204
205
206
207
208
209
...
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
...
295
296
297
298
299
300
301











302
303
304
305
306



307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
...
462
463
464
465
466
467
468
469
470
471
472
473
474

475
476
477
478
479
480
481
...
490
491
492
493
494
495
496
497
498
499
500
501






502
503
504

505
506
507
508
509
510
511
512
513


514
515
516
517
518
519
520
521
...
561
562
563
564
565
566
567


568
569
570
571
572






573
574
575
576
577
578
*/
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
#include <assert.h>






/*
** Malloc, aborting if it fails.
*/
void *safe_malloc(int nByte){
  void *x = malloc(nByte);
  if( x==0 ){
    fprintf(stderr, "failed to allocate %d bytes\n", nByte);
................................................................................
/*
** Return the first non-whitespace characters in z[]
*/
static const char *skip_space(const char *z){
  while( isspace(z[0]) ){ z++; }
  return z;
}























/*
** Return true if the input is a string literal.
*/
static int is_string_lit(const char *z){
  int nu1, nu2;
  z = next_non_whitespace(z, &nu1, &nu2);
................................................................................
  "db_setting_inop_rhs",
};

/*
** Return true if the input is an argument that is safe to use with %s
** while building an SQL statement.
*/
static int is_s_safe(const char *z){
  int len, eType;
  int i;

  /* A string literal is safe for use with %s */
  if( is_string_lit(z) ) return 1;

  /* Certain functions are guaranteed to return a string that is safe
................................................................................

  /* If the "safe-for-%s" comment appears in the argument, then
  ** let it through */
  if( strstr(z, "/*safe-for-%s*/")!=0 ) return 1;

  return 0;
}












/*
** Processing flags
*/
#define FMT_NO_S   0x00001     /* Do not allow %s substitutions */




/*
** A list of internal Fossil interfaces that take a printf-style format
** string.
*/
struct {
  const char *zFName;    /* Name of the function */
  int iFmtArg;           /* Index of format argument.  Leftmost is 1. */
  unsigned fmtFlags;     /* Processing flags */
} aFmtFunc[] = {
  { "admin_log",               1, 0 },
  { "blob_append_sql",         2, FMT_NO_S },
  { "blob_appendf",            2, 0 },
  { "cgi_debug",               1, 0 },
  { "cgi_panic",               1, 0 },
  { "cgi_printf",              1, 0 },
  { "cgi_redirectf",           1, 0 },
  { "chref",                   2, 0 },
  { "db_blob",                 2, FMT_NO_S },
  { "db_debug",                1, FMT_NO_S },
  { "db_double",               2, FMT_NO_S },
  { "db_err",                  1, 0 },
  { "db_exists",               1, FMT_NO_S },
  { "db_get_mprintf",          2, 0 },
  { "db_int",                  2, FMT_NO_S },
  { "db_int64",                2, FMT_NO_S },
  { "db_multi_exec",           1, FMT_NO_S },
  { "db_optional_sql",         2, FMT_NO_S },
  { "db_prepare",              2, FMT_NO_S },
  { "db_prepare_ignore_error", 2, FMT_NO_S },
  { "db_set_mprintf",          3, 0 },
  { "db_static_prepare",       2, FMT_NO_S },
  { "db_text",                 2, FMT_NO_S },
  { "db_unset_mprintf",        2, 0 },
  { "form_begin",              2, 0 },
  { "fossil_error",            2, 0 },
  { "fossil_errorlog",         1, 0 },
  { "fossil_fatal",            1, 0 },
  { "fossil_fatal_recursive",  1, 0 },
  { "fossil_panic",            1, 0 },
  { "fossil_print",            1, 0 },
  { "fossil_trace",            1, 0 },
  { "fossil_warning",          1, 0 },
  { "href",                    1, 0 },
  { "json_new_string_f",       1, 0 },
  { "json_set_err",            2, 0 },
  { "json_warn",               2, 0 },
  { "mprintf",                 1, 0 },
  { "socket_set_errmsg",       1, 0 },
  { "ssl_set_errmsg",          1, 0 },
  { "style_header",            1, 0 },
  { "style_set_current_page",  1, 0 },
  { "style_submenu_element",   2, 0 },
  { "style_submenu_sql",       3, 0 },
  { "webpage_error",           1, 0 },
  { "xhref",                   2, 0 },
};

/*
** Determine if the indentifier zIdent of length nIndent is a Fossil
** internal interface that uses a printf-style argument.  Return zero if not.
** Return the index of the format string if true with the left-most
** argument having an index of 1.
................................................................................
  zCopy = safe_malloc( len + 1 );
  memcpy(zCopy, zStart+1, len);
  zCopy[len] = 0;
  azArg = 0;
  nArg = 0;
  z = zCopy;
  while( z[0] ){
    len = distance_to(z, ',');
    azArg = safe_realloc((char*)azArg, (sizeof(azArg[0])+1)*(nArg+1));
    azArg[nArg++] = skip_space(z);
    if( z[len]==0 ) break;
    z[len] = 0;
    for(i=len-1; i>0 && isspace(z[i]); i--){ z[i] = 0; }

    z += len + 1;
  }
  acType = (char*)&azArg[nArg];
  if( fmtArg>nArg ){
    printf("%s:%d: too few arguments to %.*s()\n",
           zFilename, lnFCall, szFName, zFCall);
    nErr++;
................................................................................
    }else if( (k = formatArgCount(zFmt, nArg, acType))>=0
             && nArg!=fmtArg+k ){
      printf("%s:%d: too %s arguments to %.*s() "
             "- got %d and expected %d\n",
             zFilename, lnFCall, (nArg<fmtArg+k ? "few" : "many"),
             szFName, zFCall, nArg, fmtArg+k);
      nErr++;
    }else if( fmtFlags & FMT_NO_S ){
      for(i=0; i<nArg && i<k; i++){
        if( (acType[i]=='s' || acType[i]=='z' || acType[i]=='b')
         && !is_s_safe(azArg[fmtArg+i])
        ){






           printf("%s:%d: Argument %d to %.*s() not safe for SQL\n",
             zFilename, lnFCall, i+fmtArg, szFName, zFCall);
           nErr++;

        }
      }
    }
  }
  if( nErr ){
    for(i=0; i<nArg; i++){
      printf("   arg[%d]: %s\n", i, azArg[i]);
    }
  }



  free((char*)azArg);
  free(zCopy);
  return nErr;
}


/*
................................................................................
  }
  return nErr;
}

/*
** Check for format-string design rule violations on all files listed
** on the command-line.


*/
int main(int argc, char **argv){
  int i;
  int nErr = 0;
  for(i=1; i<argc; i++){






    char *zFile = read_file(argv[i]);
    nErr += scan_file(argv[i], zFile);
    free(zFile);
  }
  return nErr;
}







>
>
>
>
>







 







>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>







 







|







 







>
>
>
>
>
>
>
>
>
>
>




|
>
>
>











|

|
|
|
|
|
|
|
|

|

|
|
|
|
|
|

|
|

|
|
|
|
|
|
|
|
|
|






|
|
|
|
|
|







 







|
|
|
|
|
|
>







 







|

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








|
>
>
|







 







>
>





>
>
>
>
>
>
|





37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
...
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
...
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
...
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
...
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
...
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
...
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
*/
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
#include <assert.h>

/*
** Debugging switch
*/
static int eVerbose = 0;

/*
** Malloc, aborting if it fails.
*/
void *safe_malloc(int nByte){
  void *x = malloc(nByte);
  if( x==0 ){
    fprintf(stderr, "failed to allocate %d bytes\n", nByte);
................................................................................
/*
** Return the first non-whitespace characters in z[]
*/
static const char *skip_space(const char *z){
  while( isspace(z[0]) ){ z++; }
  return z;
}

/*
** Remove excess whitespace and nested "()" from string z.
*/
static char *simplify_expr(char *z){
  int n = (int)strlen(z);
  while( n>0 ){
    if( isspace(z[0]) ){
      z++;
      n--;
      continue;
    }
    if( z[0]=='(' && z[n-1]==')' ){
      z++;
      n -= 2;
      continue;
    }
    break;
  }
  z[n] = 0;
  return z;
}

/*
** Return true if the input is a string literal.
*/
static int is_string_lit(const char *z){
  int nu1, nu2;
  z = next_non_whitespace(z, &nu1, &nu2);
................................................................................
  "db_setting_inop_rhs",
};

/*
** Return true if the input is an argument that is safe to use with %s
** while building an SQL statement.
*/
static int is_sql_safe(const char *z){
  int len, eType;
  int i;

  /* A string literal is safe for use with %s */
  if( is_string_lit(z) ) return 1;

  /* Certain functions are guaranteed to return a string that is safe
................................................................................

  /* If the "safe-for-%s" comment appears in the argument, then
  ** let it through */
  if( strstr(z, "/*safe-for-%s*/")!=0 ) return 1;

  return 0;
}

/*
** Return true if the input is an argument that is never safe for use
** with %s.
*/
static int never_safe(const char *z){
  if( strstr(z,"/*safe-for-%s*/")!=0 ) return 0;
  if( z[0]=='P' ) return 1;  /* CGI macros like P() and PD() */
  if( strncmp(z,"cgi_param",9)==0 ) return 1;
  return 0;
}

/*
** Processing flags
*/
#define FMT_SQL   0x00001     /* Generates SQL text */
#define FMT_HTML  0x00002     /* Generates HTML text */
#define FMT_URL   0x00004     /* Generates URLs */
#define FMT_SAFE  0x00008     /* Always safe for %s */

/*
** A list of internal Fossil interfaces that take a printf-style format
** string.
*/
struct {
  const char *zFName;    /* Name of the function */
  int iFmtArg;           /* Index of format argument.  Leftmost is 1. */
  unsigned fmtFlags;     /* Processing flags */
} aFmtFunc[] = {
  { "admin_log",               1, 0 },
  { "blob_append_sql",         2, FMT_SQL },
  { "blob_appendf",            2, 0 },
  { "cgi_debug",               1, FMT_SAFE },
  { "cgi_panic",               1, FMT_SAFE },
  { "cgi_printf",              1, FMT_HTML },
  { "cgi_redirectf",           1, FMT_URL },
  { "chref",                   2, FMT_URL },
  { "db_blob",                 2, FMT_SQL },
  { "db_debug",                1, FMT_SQL },
  { "db_double",               2, FMT_SQL },
  { "db_err",                  1, 0 },
  { "db_exists",               1, FMT_SQL },
  { "db_get_mprintf",          2, 0 },
  { "db_int",                  2, FMT_SQL },
  { "db_int64",                2, FMT_SQL },
  { "db_multi_exec",           1, FMT_SQL },
  { "db_optional_sql",         2, FMT_SQL },
  { "db_prepare",              2, FMT_SQL },
  { "db_prepare_ignore_error", 2, FMT_SQL },
  { "db_set_mprintf",          3, 0 },
  { "db_static_prepare",       2, FMT_SQL },
  { "db_text",                 2, FMT_SQL },
  { "db_unset_mprintf",        2, 0 },
  { "form_begin",              2, FMT_URL },
  { "fossil_error",            2, FMT_SAFE },
  { "fossil_errorlog",         1, FMT_SAFE },
  { "fossil_fatal",            1, FMT_SAFE },
  { "fossil_fatal_recursive",  1, FMT_SAFE },
  { "fossil_panic",            1, FMT_SAFE },
  { "fossil_print",            1, FMT_SAFE },
  { "fossil_trace",            1, FMT_SAFE },
  { "fossil_warning",          1, FMT_SAFE },
  { "href",                    1, FMT_URL },
  { "json_new_string_f",       1, 0 },
  { "json_set_err",            2, 0 },
  { "json_warn",               2, 0 },
  { "mprintf",                 1, 0 },
  { "socket_set_errmsg",       1, 0 },
  { "ssl_set_errmsg",          1, 0 },
  { "style_header",            1, FMT_HTML },
  { "style_set_current_page",  1, FMT_URL },
  { "style_submenu_element",   2, FMT_URL },
  { "style_submenu_sql",       3, FMT_SQL },
  { "webpage_error",           1, FMT_SAFE },
  { "xhref",                   2, FMT_URL },
};

/*
** Determine if the indentifier zIdent of length nIndent is a Fossil
** internal interface that uses a printf-style argument.  Return zero if not.
** Return the index of the format string if true with the left-most
** argument having an index of 1.
................................................................................
  zCopy = safe_malloc( len + 1 );
  memcpy(zCopy, zStart+1, len);
  zCopy[len] = 0;
  azArg = 0;
  nArg = 0;
  z = zCopy;
  while( z[0] ){
    char cEnd;
    len = distance_to(z, ',');
    cEnd = z[len];
    z[len] = 0;
    azArg = safe_realloc((char*)azArg, (sizeof(azArg[0])+1)*(nArg+1));
    azArg[nArg++] = simplify_expr(z);
    if( cEnd==0 ) break;
    z += len + 1;
  }
  acType = (char*)&azArg[nArg];
  if( fmtArg>nArg ){
    printf("%s:%d: too few arguments to %.*s()\n",
           zFilename, lnFCall, szFName, zFCall);
    nErr++;
................................................................................
    }else if( (k = formatArgCount(zFmt, nArg, acType))>=0
             && nArg!=fmtArg+k ){
      printf("%s:%d: too %s arguments to %.*s() "
             "- got %d and expected %d\n",
             zFilename, lnFCall, (nArg<fmtArg+k ? "few" : "many"),
             szFName, zFCall, nArg, fmtArg+k);
      nErr++;
    }else if( (fmtFlags & FMT_SAFE)==0 ){
      for(i=0; i<nArg && i<k; i++){
        if( (acType[i]=='s' || acType[i]=='z' || acType[i]=='b') ){
          const char *zExpr = azArg[fmtArg+i];
          if( never_safe(zExpr) ){
            printf("%s:%d: Argument %d to %.*s() is not safe for"
                   " a query parameter\n",
               zFilename, lnFCall, i+fmtArg, szFName, zFCall);
             nErr++;
   
          }else if( (fmtFlags & FMT_SQL)!=0 && !is_sql_safe(zExpr) ){
            printf("%s:%d: Argument %d to %.*s() not safe for SQL\n",
               zFilename, lnFCall, i+fmtArg, szFName, zFCall);
             nErr++;
          }
        }
      }
    }
  }
  if( nErr ){
    for(i=0; i<nArg; i++){
      printf("   arg[%d]: %s\n", i, azArg[i]);
    }
  }else if( eVerbose>1 ){
    printf("%s:%d: %.*s() ok for %d arguments\n",
      zFilename, lnFCall, szFName, zFCall, nArg);
  }
  free((char*)azArg);
  free(zCopy);
  return nErr;
}


/*
................................................................................
  }
  return nErr;
}

/*
** Check for format-string design rule violations on all files listed
** on the command-line.
**
** The eVerbose global variable is incremented with each "-v" argument.
*/
int main(int argc, char **argv){
  int i;
  int nErr = 0;
  for(i=1; i<argc; i++){
    char *zFile;
    if( strcmp(argv[i],"-v")==0 ){
      eVerbose++;
      continue;
    }
    if( eVerbose>0 ) printf("Processing %s...\n", argv[i]);
    zFile = read_file(argv[i]);
    nErr += scan_file(argv[i], zFile);
    free(zFile);
  }
  return nErr;
}

Changes to src/login.c.

519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
      return;
    }
    if( zQS==0 ){
      zQS = "?redir=1";
    }else if( zQS[0]!=0 ){
      zQS = mprintf("?%s&redir=1", zQS);
    }
    cgi_redirectf("%s%s%s", g.zHttpsURL, P("PATH_INFO"), zQS);
    return;
  }
  sqlite3_create_function(g.db, "constant_time_cmp", 2, SQLITE_UTF8, 0,
                  constant_time_cmp_function, 0, 0);
  zUsername = P("u");
  zPasswd = P("p");
  anonFlag = g.zLogin==0 && PB("anon");







|







519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
      return;
    }
    if( zQS==0 ){
      zQS = "?redir=1";
    }else if( zQS[0]!=0 ){
      zQS = mprintf("?%s&redir=1", zQS);
    }
    cgi_redirectf("%s%T%s", g.zHttpsURL, P("PATH_INFO"), zQS);
    return;
  }
  sqlite3_create_function(g.db, "constant_time_cmp", 2, SQLITE_UTF8, 0,
                  constant_time_cmp_function, 0, 0);
  zUsername = P("u");
  zPasswd = P("p");
  anonFlag = g.zLogin==0 && PB("anon");

Changes to src/main.c.

1635
1636
1637
1638
1639
1640
1641
1642
1643
1644
1645
1646
1647
1648
1649
  if( zPathInfo && strncmp(zPathInfo,"/draft",6)==0
   && zPathInfo[6]>='1' && zPathInfo[6]<='9'
   && (zPathInfo[7]=='/' || zPathInfo[7]==0)
  ){
    int iSkin = zPathInfo[6] - '0';
    char *zNewScript;
    skin_use_draft(iSkin);
    zNewScript = mprintf("%s/draft%d", P("SCRIPT_NAME"), iSkin);
    if( g.zTop ) g.zTop = mprintf("%s/draft%d", g.zTop, iSkin);
    if( g.zBaseURL ) g.zBaseURL = mprintf("%s/draft%d", g.zBaseURL, iSkin);
    zPathInfo += 7;
    cgi_replace_parameter("PATH_INFO", zPathInfo);
    cgi_replace_parameter("SCRIPT_NAME", zNewScript);
  }








|







1635
1636
1637
1638
1639
1640
1641
1642
1643
1644
1645
1646
1647
1648
1649
  if( zPathInfo && strncmp(zPathInfo,"/draft",6)==0
   && zPathInfo[6]>='1' && zPathInfo[6]<='9'
   && (zPathInfo[7]=='/' || zPathInfo[7]==0)
  ){
    int iSkin = zPathInfo[6] - '0';
    char *zNewScript;
    skin_use_draft(iSkin);
    zNewScript = mprintf("%T/draft%d", P("SCRIPT_NAME"), iSkin);
    if( g.zTop ) g.zTop = mprintf("%s/draft%d", g.zTop, iSkin);
    if( g.zBaseURL ) g.zBaseURL = mprintf("%s/draft%d", g.zBaseURL, iSkin);
    zPathInfo += 7;
    cgi_replace_parameter("PATH_INFO", zPathInfo);
    cgi_replace_parameter("SCRIPT_NAME", zNewScript);
  }

Changes to src/zip.c.

889
890
891
892
893
894
895
896
897
898
899
900
901
902
903
    eType = ARCHIVE_SQLAR;
    zType = "SQL";
  }else{
    eType = ARCHIVE_ZIP;
    zType = "ZIP";
  }
  load_control();
  zName = mprintf("%s", PD("name",""));
  z = P("r");
  if( z==0 ) z = P("uuid");
  if( z==0 ) z = tar_uuid_from_name(&zName);
  if( z==0 ) z = "trunk";
  nName = strlen(zName);
  g.zOpenRevision = zRid = fossil_strdup(z);
  nRid = strlen(zRid);







|







889
890
891
892
893
894
895
896
897
898
899
900
901
902
903
    eType = ARCHIVE_SQLAR;
    zType = "SQL";
  }else{
    eType = ARCHIVE_ZIP;
    zType = "ZIP";
  }
  load_control();
  zName = fossil_strdup(PD("name",""));
  z = P("r");
  if( z==0 ) z = P("uuid");
  if( z==0 ) z = tar_uuid_from_name(&zName);
  if( z==0 ) z = "trunk";
  nName = strlen(zName);
  g.zOpenRevision = zRid = fossil_strdup(z);
  nRid = strlen(zRid);