Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bypassing AllowReflection = false via exceptions #1

Closed
MissyMelissa opened this issue May 4, 2017 · 6 comments
Closed

Bypassing AllowReflection = false via exceptions #1

MissyMelissa opened this issue May 4, 2017 · 6 comments
Assignees

Comments

@MissyMelissa
Copy link

It looks like it's possible to bypass AllowReflection = false via exceptions. Here is a very basic example that demonstrates the problem:

using System;
using Microsoft.ClearScript.V8;

namespace ClearScriptTest2
{
	public class Test
	{
	}

	class Program
	{
		static void Main(string[] args)
		{
			using (var engine = new V8ScriptEngine()) {
				engine.AddHostType("Console", typeof(Console));
				engine.AddHostObject("test", new Test());
				engine.AllowReflection = false;

				engine.Execute("try { test.GetType(); } catch (ex) { Console.WriteLine(ex.hostException.GetType().Assembly.FullName); }");
			}

			Console.ReadLine();
		}
	}
}

The first GetType() will throw an exception (and it is caught by javascript), but the second ex.hostException.GetType() does not throw an exception.

@ClearScriptLib
Copy link
Collaborator

Nice find! The problem is that ClearScript traps Object.GetType but not Exception.GetType.

@ClearScriptLib ClearScriptLib self-assigned this May 5, 2017
@Rene-Sackers
Copy link

@ClearScriptLib I assume you're overriding GetType on that exception, maybe check if the calling method is an override of Object.GetType using this on this?

ClearScriptLib added a commit that referenced this issue May 5, 2017
…llowReflection is false (GitHub Issue #1), renamed ReadMe.txt to Build.txt.
@ClearScriptLib
Copy link
Collaborator

@Rene-Sackers Exception.GetType isn't an override of Object.GetType; in fact, the latter isn't virtual. Exception.GetType is an implementation of an interface method (_Exception.GetType). It calls Object.GetType internally, but otherwise the methods are unrelated. Our fix simply blocks all three.

@MissyMelissa
Copy link
Author

Thank you for the quick fix!

@Rene-Sackers
Copy link

Cool! One more question though.

image

Shouldn't all these be checked/blocked, in case any are exposed through AddHostType? It might be irrelevant, only a question/suggestion :)

@ClearScriptLib
Copy link
Collaborator

@Rene-Sackers You make a good point, but most of the remaining GetType methods are reachable only if the script has already gained access to a reflection object. We also don't expect hosts to expose JScript.NET internals for scripting. AppDomain is an interesting case, but Object and Exception should cover most things likely to be made scriptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants