Fossil

Check-in [75deba73b5]
Login

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

Overview
Comment:Improved documentation of the database write protection logic. Added undocumented SQL command db_protect() and db_protect_pop() to the "sql" command. Panic on a protection stack overflow.
Downloads: Tarball | ZIP archive
Timelines: family | ancestors | descendants | both | sec2020
Files: files | file ages | folders
SHA3-256: 75deba73b5e00d6149b5f039b2622629679e5ddab835f80b639155f62585c5ed
User & Date: drh 2020-08-21 15:05:32.762
Context
2020-08-21
15:08
Add missing db_unprotect() calls to backoffice. check-in: c75dcc621b user: drh tags: sec2020
15:05
Improved documentation of the database write protection logic. Added undocumented SQL command db_protect() and db_protect_pop() to the "sql" command. Panic on a protection stack overflow. check-in: 75deba73b5 user: drh tags: sec2020
13:04
Add triggers to prevent changes to sensitive settings when PROTECT_SENSITIVE is engaged. check-in: c9b9a77d59 user: drh tags: sec2020
Changes
Unified Diff Ignore Whitespace Patch
Changes to src/db.c.
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
411
412
413
414
415
416
417
418
419
420
421
**                          protections to only X.
**
** Each of these routines pushes the previous protection mask onto
** a finite-size stack.  Each should be followed by a call to
** db_protect_pop() to pop the stack and restore the protections that
** existed prior to the call.  The protection mask stack has a limited
** depth, so take care not to next calls too deeply.

















































*/
void db_protect_only(unsigned flags){
  if( db.nProtect>=count(db.aProtect) ){
    fossil_fatal("too many db_protect() calls");
  }
  db.aProtect[db.nProtect++] = db.protectMask;
  if( (flags & PROTECT_SENSITIVE)!=0

   && (db.protectMask & PROTECT_SENSITIVE)==0


   && db.bProtectTriggers==0
  ){
    db_multi_exec(
      "CREATE TEMP TRIGGER IF NOT EXISTS protect_1"
      " BEFORE INSERT ON config WHEN protected_setting(new.name)"
      " BEGIN SELECT raise(abort,'not authorized'); END;\n"

      "CREATE TEMP TRIGGER IF NOT EXISTS protect_2"
      " BEFORE UPDATE ON config WHEN protected_setting(new.name)"
      " BEGIN SELECT raise(abort,'not authorized'); END;\n"

    );
    db.bProtectTriggers = 1;
  }
  db.protectMask = flags;
}
void db_protect(unsigned flags){
  db_protect_only(db.protectMask | flags);
}
void db_unprotect(unsigned flags){
  if( db.nProtect>=count(db.aProtect) ){
    fossil_fatal("too many db_unprotect() calls");
  }
  db.aProtect[db.nProtect++] = db.protectMask;
  db.protectMask &= ~flags;
}
void db_protect_pop(void){
  if( db.nProtect<1 ) fossil_fatal("too many db_protect_pop() calls");


  db.protectMask = db.aProtect[--db.nProtect];
}

/*
** Verify that the desired database write pertections are in place.
** Throw a fatal error if not.
*/
void db_assert_protected(unsigned flags){
  if( (flags & db.protectMask)!=flags ){
    fossil_fatal("internal security assertion fault: missing "
                 "database protection bits: %02x", flags & ~db.protectMask);
  }
}

/*
** Every Fossil database connection automatically registers the following
** overarching authenticator callback, and leaves it registered for the
** duration of the connection.  This authenticator will call any
** sub-authenticators that are registered using db_set_authorizer().
*/
static int db_top_authorizer(
  void *pNotUsed,
  int eCode,
  const char *z0,
  const char *z1,
  const char *z2,
  const char *z3
){







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


|
|


|
>
|
>
>
|
<

|
|
|
>
|
|
|
>









|
|





|
>
>




















|







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
411
412
413
414
415
416
417
418
419
420

421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
**                          protections to only X.
**
** Each of these routines pushes the previous protection mask onto
** a finite-size stack.  Each should be followed by a call to
** db_protect_pop() to pop the stack and restore the protections that
** existed prior to the call.  The protection mask stack has a limited
** depth, so take care not to next calls too deeply.
**
** About Database Write Protection
** -------------------------------
**
** This is *not* a primary means of defending the application from
** attack.  Fossil should be secure even if this mechanism is disabled.
** The purpose of database write protection is to provide an additional
** layer of defense in case SQL injection bugs somehow slip into other
** parts of the system.  In other words, database write protection is
** not primary defense but rather defense in depth.
**
** This mechanism mostly focuses on the USER table, to prevent an
** attacker from giving themselves Admin privilegs, and on the
** CONFIG table and specially "sensitive" settings such as
** "diff-command" or "editor" that if compromised by an attacker
** could lead to an RCE.
**
** By default, the USER and CONFIG tables are read-only.  Various
** subsystems that legitimately need to change those tables can
** temporarily do so using:
**
**     db_unprotect(PROTECT_xxx);
**     // make the legitmate changes here
**     db_protect_pop();
**
** Code that runs inside of reduced protections should be carefully
** reviewed to ensure that it is harmless and not subject to SQL
** injection.
**
** Read-only operations (such as many web pages like /timeline)
** can invoke db_protect(PROTECT_ALL) to effectively make the database
** read-only.  TEMP tables (which are often used for these kinds of
** pages) are still writable, however.
**
** The PROTECT_SENSITIVE protection is a subset of PROTECT_CONFIG
** that blocks changes to all of the global_config table, but only
** "sensitive" settings in the config table.  PROTECT_SENSITIVE
** relies on triggers and the protected_setting() SQL function to
** prevent changes to sensitive settings.
**
** Additional Notes
** ----------------
**
** Calls to routines like db_set() and db_unset() temporarily disable
** the PROTECT_CONFIG protection.  The assumption is that these calls
** cannot be invoked by an SQL injection and are thus safe.  Make sure
** this is the case by always using a string literal as the name argument
** to db_set() and db_unset() and friend, not a variable that might
** be compromised by an attack.
*/
void db_protect_only(unsigned flags){
  if( db.nProtect>=count(db.aProtect)-2 ){
    fossil_panic("too many db_protect() calls");
  }
  db.aProtect[db.nProtect++] = db.protectMask;
  if( (flags & PROTECT_SENSITIVE)!=0 && db.bProtectTriggers==0 ){
    /* Create the triggers needed to protect sensitive settings from
    ** being created or modified the first time that PROTECT_SENSITIVE
    ** is enabled.  Deleting a sensitive setting is harmless, so there
    ** is not trigger to block deletes.  After being created once, the
    ** triggers persist for the life of the database connection. */

    db_multi_exec(
      "CREATE TEMP TRIGGER protect_1 BEFORE INSERT ON config"
      " WHEN protected_setting(new.name) BEGIN"
      "  SELECT raise(abort,'not authorized');"
      "END;\n"
      "CREATE TEMP TRIGGER protect_2 BEFORE UPDATE ON config"
      " WHEN protected_setting(new.name) BEGIN"
      "  SELECT raise(abort,'not authorized');"
      "END;\n"
    );
    db.bProtectTriggers = 1;
  }
  db.protectMask = flags;
}
void db_protect(unsigned flags){
  db_protect_only(db.protectMask | flags);
}
void db_unprotect(unsigned flags){
  if( db.nProtect>=count(db.aProtect)-2 ){
    fossil_panic("too many db_unprotect() calls");
  }
  db.aProtect[db.nProtect++] = db.protectMask;
  db.protectMask &= ~flags;
}
void db_protect_pop(void){
  if( db.nProtect<1 ){
    fossil_panic("too many db_protect_pop() calls");
  }
  db.protectMask = db.aProtect[--db.nProtect];
}

/*
** Verify that the desired database write pertections are in place.
** Throw a fatal error if not.
*/
void db_assert_protected(unsigned flags){
  if( (flags & db.protectMask)!=flags ){
    fossil_fatal("internal security assertion fault: missing "
                 "database protection bits: %02x", flags & ~db.protectMask);
  }
}

/*
** Every Fossil database connection automatically registers the following
** overarching authenticator callback, and leaves it registered for the
** duration of the connection.  This authenticator will call any
** sub-authenticators that are registered using db_set_authorizer().
*/
int db_top_authorizer(
  void *pNotUsed,
  int eCode,
  const char *z0,
  const char *z1,
  const char *z2,
  const char *z3
){
437
438
439
440
441
442
443


444
445
446
447
448
449
450
      }else if( (db.protectMask & PROTECT_READONLY)!=0
                && sqlite3_stricmp(z2,"temp")!=0 ){
        rc = SQLITE_DENY;
      }
      break;
    }
    case SQLITE_DROP_TEMP_TRIGGER: {


      rc = SQLITE_DENY;
      break;
    }
  }
  if( db.xAuth && rc==SQLITE_OK ){
    rc = db.xAuth(db.pAuthArg, eCode, z0, z1, z2, z3);
  }







>
>







492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
      }else if( (db.protectMask & PROTECT_READONLY)!=0
                && sqlite3_stricmp(z2,"temp")!=0 ){
        rc = SQLITE_DENY;
      }
      break;
    }
    case SQLITE_DROP_TEMP_TRIGGER: {
      /* Do not allow the triggers that enforce PROTECT_SENSITIVE
      ** to be dropped */
      rc = SQLITE_DENY;
      break;
    }
  }
  if( db.xAuth && rc==SQLITE_OK ){
    rc = db.xAuth(db.pAuthArg, eCode, z0, z1, z2, z3);
  }
Changes to src/printf.c.
1146
1147
1148
1149
1150
1151
1152

1153
1154


1155
1156
1157
1158
1159
1160
1161
  }
  fossil_errorlog("panic: %s", z);
  rc = fossil_print_error(rc, z);
  abort();
  exit(rc);
}
NORETURN void fossil_fatal(const char *zFormat, ...){

  char *z;
  int rc = 1;


  va_list ap;
  mainInFatalError = 1;
  va_start(ap, zFormat);
  z = vmprintf(zFormat, ap);
  va_end(ap);
  rc = fossil_print_error(rc, z);
  fossil_free(z);







>


>
>







1146
1147
1148
1149
1150
1151
1152
1153
1154
1155
1156
1157
1158
1159
1160
1161
1162
1163
1164
  }
  fossil_errorlog("panic: %s", z);
  rc = fossil_print_error(rc, z);
  abort();
  exit(rc);
}
NORETURN void fossil_fatal(const char *zFormat, ...){
  static int once = 0;
  char *z;
  int rc = 1;
  if( once ) exit(1);
  once = 1;
  va_list ap;
  mainInFatalError = 1;
  va_start(ap, zFormat);
  z = vmprintf(zFormat, ap);
  va_end(ap);
  rc = fossil_print_error(rc, z);
  fossil_free(z);
Changes to src/sqlcmd.c.
151
152
153
154
155
156
157


































158
159
160
161
162
163
164
                          sqlcmd_compress, 0, 0);
  sqlite3_create_function(db, "decompress", 1, SQLITE_UTF8, 0,
                          sqlcmd_decompress, 0, 0);
  sqlite3_create_function(db, "gather_artifact_stats", 0, SQLITE_UTF8, 0,
                          sqlcmd_gather_artifact_stats, 0, 0);
  return SQLITE_OK;
}



































/*
** This is the "automatic extension" initializer that runs right after
** the connection to the repository database is opened.  Set up the
** database connection to be more useful to the human operator.
*/
static int sqlcmd_autoinit(







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







151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
                          sqlcmd_compress, 0, 0);
  sqlite3_create_function(db, "decompress", 1, SQLITE_UTF8, 0,
                          sqlcmd_decompress, 0, 0);
  sqlite3_create_function(db, "gather_artifact_stats", 0, SQLITE_UTF8, 0,
                          sqlcmd_gather_artifact_stats, 0, 0);
  return SQLITE_OK;
}

/*
** Undocumented test SQL functions:
**
**     db_protect(X)
**     db_protect_pop(X)
**
** These invoke the corresponding C routines.  Misuse may result in
** an assertion fault.
*/
static void sqlcmd_db_protect(
  sqlite3_context *context,
  int argc,
  sqlite3_value **argv
){
  unsigned mask = 0;
  const char *z = (const char*)sqlite3_value_text(argv[0]);
  if( sqlite3_stricmp(z,"user")==0 )      mask |= PROTECT_USER;
  if( sqlite3_stricmp(z,"config")==0 )    mask |= PROTECT_CONFIG;
  if( sqlite3_stricmp(z,"sensitive")==0 ) mask |= PROTECT_SENSITIVE;
  if( sqlite3_stricmp(z,"readonly")==0 )  mask |= PROTECT_READONLY;
  if( sqlite3_stricmp(z,"all")==0 )       mask |= PROTECT_ALL;
  db_protect(mask);
}
static void sqlcmd_db_protect_pop(
  sqlite3_context *context,
  int argc,
  sqlite3_value **argv
){
  db_protect_pop();
}




/*
** This is the "automatic extension" initializer that runs right after
** the connection to the repository database is opened.  Set up the
** database connection to be more useful to the human operator.
*/
static int sqlcmd_autoinit(
191
192
193
194
195
196
197






198
199
200
201
202
203
204
    sqlite3_exec(db, zSql, 0, 0, 0);
    sqlite3_free(zSql);
  }
  /* Arrange to trace close operations so that static prepared statements
  ** will get cleaned up when the shell closes the database connection */
  if( g.fSqlTrace ) mTrace |= SQLITE_TRACE_PROFILE;
  sqlite3_trace_v2(db, mTrace, db_sql_trace, 0);






  return SQLITE_OK;
}

/*
** atexit() handler that cleans up global state modified by this module.
*/
static void sqlcmd_atexit(void) {







>
>
>
>
>
>







225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
    sqlite3_exec(db, zSql, 0, 0, 0);
    sqlite3_free(zSql);
  }
  /* Arrange to trace close operations so that static prepared statements
  ** will get cleaned up when the shell closes the database connection */
  if( g.fSqlTrace ) mTrace |= SQLITE_TRACE_PROFILE;
  sqlite3_trace_v2(db, mTrace, db_sql_trace, 0);
  db_protect_only(PROTECT_NONE);
  sqlite3_set_authorizer(db, db_top_authorizer, db);
  sqlite3_create_function(db, "db_protect", 1, SQLITE_UTF8, 0,
                          sqlcmd_db_protect, 0, 0);
  sqlite3_create_function(db, "db_protect_pop", 0, SQLITE_UTF8, 0,
                          sqlcmd_db_protect_pop, 0, 0);
  return SQLITE_OK;
}

/*
** atexit() handler that cleans up global state modified by this module.
*/
static void sqlcmd_atexit(void) {