search

jglisson / elmah

Automatically exported from code.google.com/p/elmah
Apache License 2.0
0 stars 0 forks source link

JScript error filter does not work in Full trust #278

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago 
What steps will reproduce the problem?
1. Create an ASP.NET Web Application and configure both ErrorLogModule and 
ErrorMailModule in Web.config.
2. Try to hack the website using something like "/?<script>" (to throw an 
HttpRequestValidationException).
3. Verify the error is logged as expected and an email is received from ELMAH.
4. Specify the following filter in Web.config:

  <elmah>
    ...
    <errorFilter>
      <test>
        <jscript>
          <expression>
            <![CDATA[
              // @assembly mscorlib
              // @assembly System.Web, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
              // @import System.Web

              // Do not send email notification when hackers attempt something
              // like "http://.../?<script>"
              FilterSourceType.Name == "ErrorMailModule"
                && BaseException instanceof HttpRequestValidationException
              ]]>
          </expression>
        </jscript>
      </test>
    </errorFilter>
  </elmah>

5. Change the website to run in Medium trust:

  <system.web>
    <trust level="Medium" originUrl=".*" />
    ...
  </system.web>

6. Repeat step 2.
7. Verify the error is logged as expected but the email is *not* received (so 
far, so good).
8. Change the website to run in Full trust:

  <system.web>
    <trust level="Full" />
    ...
  </system.web>

9. Repeat step 2.
10. Verify the error is logged as expected *and* the email is received (even 
though it should have been prevented by the filter).

It appears there is some sort of caching bug in FullTrustEvaluationStrategy 
when using FilterSourceType.Name (in other words, FilterSourceType.Name is not 
evaluated the second time through when the "ErrorMailModule" context object is 
passed to the Eval method).

What is the expected output? What do you see instead?

The filter should work the same in Full trust and Medium trust (meaning the 
error should be logged, but an email should not be sent from ELMAH due to the 
filter). Instead of correctly applying the filter in the Full trust 
configuration, FilterSourceType.Name is not evaluated as expected (it appears 
to be evaluated as "ErrorLogModule" even though the context object is 
"ErrorMailModule").

What version of the product are you using? On what operating system?

I have verified this issue occurs in the latest released version of ELMAH (1.2 
SP1) as well as the latest rev (a53a22fcf694 - 2.0.11227.2305).

OS = Windows Server 2008 R2 SP1

Please provide any additional information below.

I will attach a repro solution in a moment (as soon as I have an issue #).

Original issue reported on code.google.com by jjame...@technologytoolbox.com on 26 Feb 2012 at 10:52

GoogleCodeExporter commented 8 years ago 
Attaching repro solution

Original comment by jjame...@technologytoolbox.com on 26 Feb 2012 at 10:54

Attachments:

GoogleCodeExporter commented 8 years ago 
Issue 277 has been merged into this issue.

Original comment by jamesdriscoll71 on 27 Feb 2012 at 2:23

GoogleCodeExporter commented 8 years ago 
Issue caused by DataBinder.cs calling System.Web.UI.DataBinder.Eval in medium 
trust - it doesn't allow access to AssertionHelperContext.FilterSourceType.Name

This can be overcome by adding an additional property:

            public string FilterSourceTypeName
            {
                get { return FilterSourceType.Name; }
            }

Original comment by jamesdriscoll71 on 27 Feb 2012 at 2:34

GoogleCodeExporter commented 8 years ago 
[deleted comment]
GoogleCodeExporter commented 8 years ago 
Comment #3 (http://code.google.com/p/elmah/issues/detail?id=278#c3) appears to 
be the solution for  issue 277 . Issue 278 is related to a caching issue in the 
FullTrustEvaluationStrategy implementation.

It's possible that adding an additional property (FilterSourceTypeName) would 
also workaround the bug in FullTrustEvaluationStrategy. However, that would 
still leave open the possibility of encountering this issue when trying to 
access other properties.

Original comment by jjame...@technologytoolbox.com on 27 Feb 2012 at 3:44

GoogleCodeExporter commented 8 years ago 
This appears to work:

<errorFilter>
      <test>
        <jscript>
          <expression>
            <![CDATA[
              // @assembly mscorlib
              // @assembly System.Web, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
              // @assembly Elmah
              // @import System.Web
              // @import Elmah

              // Do not send email notification when hackers attempt something
              // like "http://.../?<script>
              FilterSource instanceof ErrorMailModule
              && BaseException instanceof HttpRequestValidationException
              ]]>
          </expression>
        </jscript>
      </test>
    </errorFilter>

But again, the underlying problem needs more work!

Original comment by jamesdriscoll71 on 28 Feb 2012 at 8:03

GoogleCodeExporter commented 8 years ago 
I have been able to reproduce the reported issue. Unfortunately, it appears 
that the JScript-based assertion produces unreliable results under full trust 
but works as expected under partial trust. This is true for the original 
JScript expression reported and the workaround proposed in comment #6.

I have attached a stand-alone console application source that exercises the 
JScript expression under full (assuming app starts in full) and partial trust. 
I have also attached the output from testing against ELMAH 1.2 SP1 release. 
Both files are also posted online[1].

As noted, the problem appears to be with the optimized code path 
(FullTrustEvaluationStrategy) used under full trust. I don't have further 
information at this time on why it doesn't work as intended. There is also no 
workaround or option to force the partial trust evaluation strategy under full 
trust. ELMAH does however support custom assertions[2] to be written and 
plugged-in and one such custom assertion could be based off 
Elmah.Assertions.JScriptAssertion's[3] partial trust evaluation strategy.

[1] https://gist.github.com/1935713
[2] 
http://code.google.com/p/elmah/wiki/ErrorFiltering#Writing_Your_Own_Assertion
[3] 
http://elmah.googlecode.com/svn/tags/REL-1.2-SP1/src/Elmah/Assertions/JScriptAss
ertion.cs

Original comment by azizatif on 28 Feb 2012 at 11:03

Attachments:

GoogleCodeExporter commented 8 years ago 
James and Atif,

Thanks for the comments and potential workarounds. Since my site has to run in 
Medium trust, I ended up going with the JavaScript filter (since that approach 
works as expected in Medium trust).

I really appreciate the work you guys have put into making ELMAH what it is 
today. "Fantastic" is the first word that comes to mind when I think of ELMAH.

FYI, I just finished writing up a blog post explaining more about my adventures 
with ELMAH last weekend:

http://www.technologytoolbox.com/blog/jjameson/archive/2012/02/28/filter-elmah-e
mail-messages-to-avoid-getting-spammed-by-hackers.aspx

Thanks again for the prompt responses.

Original comment by jjame...@technologytoolbox.com on 29 Feb 2012 at 3:37

GoogleCodeExporter commented 8 years ago 
It seems to me that the problem revolves around the way that the JScript VSA 
engine evaluates the with (context) statement. For some reason it will happily 
evaluate FilterSourceType each time, but will have cached FilterSourceType.Name 
from the first evaluation!

I think I've got a solution to the problem (using the attached patch to 
JScriptAssertion.cs), but it does involve a breaking change in the way that the 
JScript assertion is declared. In this instance, the assertion would become:

    <errorFilter>
      <test>
        <jscript>
          <expression>
            <![CDATA[
              // @assembly mscorlib
              // @assembly System.Web, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
              // @assembly Elmah
              // @import System.Web
              // @import Elmah

              // Do not send email notification when hackers attempt something
              // like "http://.../?<script>
              context.FilterSourceType.Name == "ErrorMailModule"
              && context.BaseException instanceof HttpRequestValidationException
              ]]>
          </expression>
        </jscript>
      </test>
    </errorFilter>

Note how they key fields are being prefixed with context.
I've run this through a modified version of Atif's test code from comment #7 
(http://code.google.com/p/elmah/issues/detail?id=278#c7) and it appears to work 
as expected.

Original comment by jamesdriscoll71 on 29 Feb 2012 at 8:44

Attachments:

GoogleCodeExporter commented 8 years ago 
> FYI, I just finished writing up a blog post 
> explaining more about my adventures with ELMAH 
> last weekend:

Thanks for the immaculately detailed blog posts! I've added them to the 
WebBase[1] wiki.

[1] http://code.google.com/p/elmah/wiki/WebBase
[2] 
http://code.google.com/p/elmah/source/detail?r=b0f348adbde49ebc1e739e50943dad166
7e9cd5b&repo=wiki

Original comment by azizatif on 2 Mar 2012 at 10:18

GoogleCodeExporter commented 8 years ago 
James, the patch isn't actually necessary because it just so happens that the 
JScript assertion already has a hidden $context available but which I meant to 
keep reserved/undocumented for future compatibility. $context is brought into 
scope automatically by issuing a with statement! If the with statement is 
what's causing the problem then a workaround would be to simply specify it 
explicitly like this:

  // @assembly mscorlib
  // @assembly System.Web, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
  // @import System.Web
  // Do not send email notification when hackers attempt something
  // like http://.../?<script>
  $context.FilterSourceType.Name == 'ErrorMailModule'
  && $context.BaseException instanceof HttpRequestValidationException

I've tested this and it works identically under full and partial trust.

Original comment by azizatif on 2 Mar 2012 at 10:29

Attachments:

GoogleCodeExporter commented 8 years ago 
Cool!!
What do you think about the change to use Closure instead of 
LateBinding.CallValue??
The code looks simpler to me and I'm thinking it might be slightly more 
efficient too!
All it would need is to add in the with statement.

Original comment by jamesdriscoll71 on 2 Mar 2012 at 10:48

GoogleCodeExporter commented 8 years ago 
James, the JScript engine is mostly undocumented. I reverse engineered most of 
its use by writing js code, compiling using the Managed JScript compiler 
(jsc.exe) and then decompiling the MSIL and studying the calls into 
Microsoft.JScript assembly. Obviously, it's a fragile method as evidenced by 
this full trust issue.

The closure code certainly looks simpler but I wanted to avoid the use of eval, 
which is why I went with Function construction. I'm assuming the latter 
compiles the body supplied as a string is more restricted than a full and blind 
eval. There's also no guarantee that we might discover other side effects later 
down the road with this change.

At this stage, we know that the with statement seems to be causing trouble and 
if explicitly referring to the context parameter helps then I think the best 
and safest course of action is to do just that. Perhaps the context parameter 
can be additionally shortened to just $. Since the fix would be fairly small, 
we could afford to schedule this for 1.2 SP2.

Original comment by azizatif on 3 Mar 2012 at 2:45

GoogleCodeExporter commented 8 years ago 
Attached is a patch demonstrating the idea in comment #13 along with the 
updated test program and output.

Original comment by azizatif on 3 Mar 2012 at 3:43

Attachments:

GoogleCodeExporter commented 8 years ago

Original comment by azizatif on 3 Mar 2012 at 6:01

GoogleCodeExporter commented 8 years ago 
This issue was closed by revision fe7a9372d177.

Original comment by azizatif on 3 Mar 2012 at 6:25

GoogleCodeExporter commented 8 years ago 
James, I think the same fix will need to be applied to the web.config 
(transform) in the elmah.filtering.sample NuGet package.

http://code.google.com/p/elmah/source/browse/nuget/Transforms/Elmah.Filtering.Sa
mple.web.config.transform?repo=pkg

Original comment by azizatif on 3 Mar 2012 at 6:32

GoogleCodeExporter commented 8 years ago 
Definitely... I'll get on the case!

Re comment #13... fair enough... though the Eval is only called the once!
After that, I think we're into the world of invoke a delegate - which I believe 
should be pretty consistent.

BTW - got inspiration for the Closure code from 
http://www.west-wind.com/weblog/posts/2007/Feb/14/Evaluating-JavaScript-code-fro
m-C

Original comment by jamesdriscoll71 on 3 Mar 2012 at 7:04

GoogleCodeExporter commented 8 years ago 
This issue was closed by revision aed6297b1cfc.

Original comment by azizatif on 3 Mar 2012 at 7:18

GoogleCodeExporter commented 8 years ago 
James, I am not as concerned about the performance of eval as about its 
unsafety. Using eval to define a function means that someone can close the 
definition and do other things in the global context intentionally or 
unintentionally, a bit like a SQL injection. I would rather err on the safe 
side in full trust mode. In the partial trust case, the only way I found to 
make the JScript assertion work was through eval (unless you know of another?). 
However, the assumption there was that the whole engine would be running under 
a restricted security sandbox. Using the Function constructor seems safer 
because you cannot go outside the body of the function.

Original comment by azizatif on 3 Mar 2012 at 7:49