Hide mw.makeProtectedEnvFuncs from modules

Allowing a module to call mw.makeProtectedEnvFuncs() lets it bypass the
allowEnvFuncs setting. It can also be used to manipulate the global
tables that other modules' sandboxes will be copied from.

And for paranoia's sake, let's tighten up what setfenv is allowed to
set. This requires changing a unit test, because it is no longer
sane to do something like

 env.setfenv, env.getfenv = mw.makeProtectedEnvFuncs( { [env] = true }, {} )

Nothing real does this, it was only in the unit test.

Change-Id: I8e0d83bb0980ee869af3ac4413afd211717ca92f

engines/LuaCommon/lualib/mw.lua

@@ -136,6 +136,9 @@ function mw.executeModule( chunk )
 	local env = mw.clone( _G )
 	makePackageModule( env )
 
+	-- This is unsafe
+	env.mw.makeProtectedEnvFuncs = nil
+
 	if allowEnvFuncs then
 		env.setfenv, env.getfenv = mw.makeProtectedEnvFuncs( {[_G] = true}, {} )
 	else
@@ -188,6 +191,10 @@ function mw.makeProtectedEnvFuncs( protectedEnvironments, protectedFunctions )
 			if protectedFunctions[func] then
 				error( "'setfenv' cannot be called on a protected function", 2 )
 			end
+			local env = old_getfenv( func )
+			if env == nil or protectedEnvironments[ env ] then
+				error( "'setfenv' cannot set the requested environment, it is protected", 2 )
+			end
 			old_setfenv( func, newEnv )
 		else
 			error( "'setfenv' can only be called with a function or integer as the first argument", 2 )

tests/engines/LuaCommon/CommonTest.php

@@ -45,6 +45,12 @@ class Scribunto_LuaCommonTests extends Scribunto_LuaEngineTestBase {
 
 		// Note this depends on every iteration of the data provider running with a clean parser
 		$this->getEngine()->getParser()->getOptions()->setExpensiveParserFunctionLimit( 10 );
+
+		// Some of the tests need this
+		$interpreter = $this->getEngine()->getInterpreter();
+		$interpreter->callFunction(
+			$interpreter->loadString( 'mw.makeProtectedEnvFuncsForTest = mw.makeProtectedEnvFuncs', 'fortest' )
+		);
 	}
 
 	function getTestModules() {

tests/engines/LuaCommon/CommonTests.lua

@@ -52,7 +52,7 @@ function test.setfenv3()
 	local function jailbreak()
 		setfenv( 2, {} )
 	end
-	local new_setfenv, new_getfenv = mw.makeProtectedEnvFuncs( { [_G] = true }, {} )
+	local new_setfenv, new_getfenv = mw.makeProtectedEnvFuncsForTest( { [_G] = true }, {} )
 	setfenv( jailbreak, {setfenv = new_setfenv} )
 	jailbreak()
 end
@@ -62,17 +62,17 @@ function test.setfenv4()
 	-- environment. It's assumed that any higher-level environment will protect
 	-- itself with its own setfenv wrapper, so this succeeds.
 	local function level3()
+		local protected = {setfenv = setfenv, getfenv = getfenv, mw = mw}
 		local function level2()
-			local env = {setfenv = setfenv}
 			local function level1()
 				setfenv( 3, {} )
 			end
 
+			local env = {}
+			env.setfenv, env.getfenv = mw.makeProtectedEnvFuncsForTest(
+				{[protected] = true}, {} )
 			setfenv( level1, env )()
 		end
-		local protected = {mw = mw}
-		protected.setfenv, protected.getfenv = mw.makeProtectedEnvFuncs(
-			{[protected] = true}, {} )
 		setfenv( level2, protected )()
 	end
 	local unprotected = {setfenv = setfenv, getfenv = getfenv, mw = mw}
@@ -85,7 +85,7 @@ function test.setfenv5()
 	local function allowed()
 		(function() setfenv( 2, {} ) end )()
 	end
-	local new_setfenv, new_getfenv = mw.makeProtectedEnvFuncs( { [_G] = true }, {} )
+	local new_setfenv, new_getfenv = mw.makeProtectedEnvFuncsForTest( { [_G] = true }, {} )
 	setfenv( allowed, {setfenv = new_setfenv} )()
 	return 'ok'
 end
@@ -95,7 +95,7 @@ function test.setfenv6()
 	local function jailbreak()
 		setfenv( target, {} )
 	end
-	local new_setfenv, new_getfenv = mw.makeProtectedEnvFuncs( {}, { [target] = true } )
+	local new_setfenv, new_getfenv = mw.makeProtectedEnvFuncsForTest( {}, { [target] = true } )
 	setfenv( jailbreak, {setfenv = new_setfenv} )()
 end