about summary refs log tree commit diff stats
diff options
context:
space:
mode:
authorKartik K. Agaram <vc@akkartik.com>2022-03-07 15:40:28 -0800
committerKartik K. Agaram <vc@akkartik.com>2022-03-07 15:40:28 -0800
commitf268015ac089d26f9bb957dad53e94ae78a3d8a8 (patch)
treec5a4cbbdb1b7bed93be954c257d5df8615188d2e
parentcfb7cff4c111959779710d0c8a9a89f108a9e8d1 (diff)
downloadteliva-f268015ac089d26f9bb957dad53e94ae78a3d8a8.tar.gz
fix the security vulnerability
We now have a notion of libraries that we load after app code, to
prevent them from getting overridden.

Should I just load all libraries after the app? There might be value in
allowing apps to override library functions. Disallowing that too much
may be going against Lua's dynamic nature.
-rw-r--r--attack.tlv12
-rw-r--r--src/lua.c6
2 files changed, 8 insertions, 10 deletions
diff --git a/attack.tlv b/attack.tlv
index 9dbbc91..195d6bc 100644
--- a/attack.tlv
+++ b/attack.tlv
@@ -380,7 +380,7 @@
     >end
 - __teliva_timestamp: original
   doc:blurb:
-    >An example app that does something malicious.
+    >A manual test for an arcane sandboxing scenario.
     >
     >Steps to reproduce:
     >  - browse to 'main' and see that it's just trying to write
@@ -396,18 +396,14 @@
     >    return caller == 'main'
     >    ```
     >  - hit ctrl-x twice to return to the app
-    >  - notice an error saying "wrote to malicious file!!"
     >
-    >The gap here is that Teliva refuses to load keys from .tlv
-    >apps that exist. However, we don't actually enforce that
-    >keys in .tlv apps only create definitions corresponding to
-    >the key. So simply lying about the definition being
-    >modified suffices to get past all our existing protections.
+    >A failure to sandbox this app is indicated by the error,
+    >"wrote to malicious file!!"
 - __teliva_timestamp: original
   foo:
     >-- maliciously write to a primitive Teliva's permission system cares about
     >-- it's important that this definition is camouflaged as a definition of
-    >-- 'foo'.
+    >-- 'foo' (something different from 'start_writing').
     >function start_writing(fs, filename)
     >  local outfile = io.open('malicious_file', 'w')
     >  if outfile then
diff --git a/src/lua.c b/src/lua.c
index b853226..263d89c 100644
--- a/src/lua.c
+++ b/src/lua.c
@@ -213,6 +213,7 @@ static int pmain (lua_State *L) {
   globalL = L;
   if (argv[0] && argv[0][0]) progname = argv[0];
   lua_gc(L, LUA_GCSTOP, 0);  /* stop collector during initialization */
+  /* Libraries that can be over-ridden */
   luaL_openlibs(L);
   status = dorequire(L, "src/lcurses/curses.lua", "curses");
   if (status != 0) return 0;
@@ -238,13 +239,14 @@ static int pmain (lua_State *L) {
   if (status != 0) return 0;
   status = dorequire(L, "src/task.lua", "task");
   if (status != 0) return 0;
-  status = dorequire(L, "src/file.lua", "file");
-  if (status != 0) return 0;
   lua_gc(L, LUA_GCRESTART, 0);
   s->status = handle_luainit(L);
   if (s->status != 0) return 0;
   s->status = load_image(L, argv, 1);
   if (s->status != 0) return 0;
+  /* Security-sensitive libraries that cannot be over-ridden */
+  status = dorequire(L, "src/file.lua", "file");
+  if (status != 0) return 0;
   /* call main() */
   lua_getglobal(L, "spawn_main");
   s->status = docall(L, 0, 1);