View Issue Details

IDProjectCategoryView StatusLast Update
0000543LDMud 3.3Efunspublic2008-07-10 04:00
ReporterGnomi Assigned ToGnomi  
PrioritynormalSeverityfeatureReproducibilityalways
Status resolvedResolutionfixed 
Platformi686OSDebian GNU/LinuxOS Version4.0
Product Version3.3.716 
Fixed in Version3.3.717 
Summary0000543: Mudlib controlled sqlite pragmas
DescriptionThis is a patch that calls master->privilege_violation for each sqlite pragma, so that the mudlib can decide whether to allow or disallow. The current behavior is to allow 'pragma synchronous' and disallow all other. (This is a TODO item in pkg-sqlite.c.)

In UNItopia we used this patch for a year. The only drawback I can see is that it makes secure_apply_error visible to all the other files, but I wanted to avoid duplicate code and I do need such a function for error handling.
TagsNo tags attached.

Activities

2008-07-01 03:10

 

sqlite_pragma.diff (5,184 bytes)   
Index: 3.3sqlite/src/pkg-sqlite.c
===================================================================
--- 3.3sqlite/src/pkg-sqlite.c	(Revision 2314)
+++ 3.3sqlite/src/pkg-sqlite.c	(Arbeitskopie)
@@ -74,7 +74,7 @@
 /* The list of database connections.
  */ 
 static sqlite_dbs_t *head = NULL;
-  
+
 /*-------------------------------------------------------------------------*/
 static sqlite_dbs_t *
 find_db (object_t * obj) 
@@ -150,29 +150,83 @@
 
 /*-------------------------------------------------------------------------*/
 static int
-my_sqlite3_authorizer(void * data, int what, const char* arg1, const char* arg2,
+my_sqlite3_authorizer (void * data, int what, const char* arg1, const char* arg2,
         const char* dbname, const char* view)
 
 /* Callback function for SQLite to handle authorizations.
  */
 
 {
-    /* TODO: Check them via privilege_violation resp. valid_write.
-             (Don't know, whether sqlite can handle longjmps out of
-             its code in case of an error...)
-    */
-
+    struct error_recovery_info error_recovery_info;
+    svalue_t *save_sp, sarg1, sarg2;
+    struct control_stack *save_csp;
+    int val;
+    
     switch(what)
     {
         case SQLITE_PRAGMA:
-            if(!strcasecmp(arg1, "synchronous"))
-                return SQLITE_OK;
-            return SQLITE_DENY;
+            /* PRAGMA name [ = value ]
+             * PRAGMA function(arg)
+             *
+             *   arg1: name/function
+             *   arg2: value/arg
+             *   dbname/view: NULL
+             */
+            
+            error_recovery_info.rt.last = rt_context;
+            error_recovery_info.rt.type = ERROR_RECOVERY_APPLY;
+            rt_context = (rt_context_t *)&error_recovery_info;
 
+            save_sp = inter_sp;
+            save_csp = csp;
+            sarg1.type = T_INVALID;
+            sarg2.type = T_INVALID;
+
+            if (setjmp(error_recovery_info.con.text))
+            {
+                secure_apply_error(save_sp, save_csp, MY_FALSE);
+                val = SQLITE_DENY;
+            }
+            else
+            {
+                if(arg1)
+                    put_c_string(&sarg1, arg1);
+                else
+                    put_number(&sarg1, 0);
+                
+                if(arg2)
+                    put_c_string(&sarg2, arg2);
+                else
+                    put_number(&sarg2, 0);
+
+                if(privilege_violation2(STR_SQLITE_PRAGMA, &sarg1, &sarg2, inter_sp))
+                    val = SQLITE_OK;
+                else
+                    val = SQLITE_DENY;
+            }
+
+            free_svalue(&sarg1);
+            sarg1.type = T_INVALID;
+            free_svalue(&sarg2);
+            sarg2.type = T_INVALID;
+
+            rt_context = error_recovery_info.rt.last;
+
+            return val;
+
         case SQLITE_ATTACH:
-        case SQLITE_DETACH:
+            /* ATTACH "filename" AS "dbname"
+             *
+             *   arg1: filename
+             *   arg2, dbname, view: NULL
+             */
+
+            /* SQLite3 doesn't allow the filename to be changed,
+             * but at least we must convert an absolute pathname
+             * to a relative one. So we have to deactivate it...
+             */
             return SQLITE_DENY;
-    
+
         default:
             return SQLITE_OK;
     }
@@ -323,7 +377,7 @@
     db = find_db (current_object);
     if (!db)
         errorf("The current object doesn't have a database open.\n");
-
+    
     err = sqlite3_prepare(db->db, get_txt(argp->u.str), mstrsize(argp->u.str),
         &stmt, &tail);
     if(err)
Index: 3.3sqlite/src/string_spec
===================================================================
--- 3.3sqlite/src/string_spec	(Revision 2314)
+++ 3.3sqlite/src/string_spec	(Arbeitskopie)
@@ -232,6 +232,7 @@
 
 #ifdef USE_SQLITE
 SQLITE_OPEN     "sl_open"
+SQLITE_PRAGMA   "sqlite_pragma"
 #endif
 
 /***************************************************************************/
Index: 3.3sqlite/src/interpret.c
===================================================================
--- 3.3sqlite/src/interpret.c	(Revision 2314)
+++ 3.3sqlite/src/interpret.c	(Arbeitskopie)
@@ -17093,7 +17093,7 @@
 } /* apply() */
 
 /*-------------------------------------------------------------------------*/
-static void
+void
 secure_apply_error ( svalue_t *save_sp, struct control_stack *save_csp
                    , Bool clear_costs)
 
Index: 3.3sqlite/src/interpret.h
===================================================================
--- 3.3sqlite/src/interpret.h	(Revision 2314)
+++ 3.3sqlite/src/interpret.h	(Arbeitskopie)
@@ -183,6 +183,7 @@
 extern int get_line_number_if_any(string_t **name);
 extern void reset_machine(Bool first);
 extern svalue_t *secure_apply(string_t *fun, object_t *ob, int num_arg);
+extern void secure_apply_error(svalue_t *save_sp, struct control_stack *save_csp, Bool clear_costs);
 extern svalue_t *apply_master_ob(string_t *fun, int num_arg, Bool external);
 #define apply_master(fun, num_arg) apply_master_ob(fun, num_arg, MY_FALSE)
 #define callback_master(fun, num_arg) apply_master_ob(fun, num_arg, MY_TRUE)
sqlite_pragma.diff (5,184 bytes)   

zesstra

2008-07-08 17:45

administrator   ~0000688

Seems to me OK as well. Did you know if secure_apply_error() was actually inlined until now or did you recognize any impact? But anyway I guess, it would be neglectable.
I think, the thing missing is the documentation (doc/master/privilege_violation). ;-)

Gnomi

2008-07-09 10:39

manager   ~0000703

I doesn't look like it was inlined before. But errors in master applies should be rare, so it doesn't matter much.

Oh, documentation, right. I'll try. :-)

Gnomi

2008-07-10 04:00

manager   ~0000712

Committed as r2378.

Issue History

Date Modified Username Field Change
2008-07-01 03:10 Gnomi New Issue
2008-07-01 03:10 Gnomi Status new => assigned
2008-07-01 03:10 Gnomi Assigned To => Gnomi
2008-07-01 03:10 Gnomi File Added: sqlite_pragma.diff
2008-07-08 17:45 zesstra Note Added: 0000688
2008-07-09 10:39 Gnomi Note Added: 0000703
2008-07-10 04:00 Gnomi Status assigned => resolved
2008-07-10 04:00 Gnomi Fixed in Version => 3.3.717
2008-07-10 04:00 Gnomi Resolution open => fixed
2008-07-10 04:00 Gnomi Note Added: 0000712