Exception filters considered dangerous

Shawn Farkas recently posted an interesting note about impersonation. He points out that the following code...

WindowsImpersonationContext ctx = identity.Impersonate();
try { DoWork(); } finally { ctx.Undo(); }

... has a subtle hole - if an untrusted caller is above you on the stack somewhere, and if he can cause DoWork to throw an exception for some reason (perhaps by temporarily exhausting some resource that DoWork needs), he can run arbitrary code in the context of an exception filter (e.g., the Where clause in VB).

He's right. This is very subtle. But it applies to many more situations than just impersonation. If you are being called by untrusted code, then you have to assume that this trick can be used to run arbitrary malicious code in the middle of any of your routines. What happens if your state is inconsistent at the moment an exception filter runs? We often use finally blocks to clean things up before we leave the “safety” of a private function. Now we discover that our classes need to worry about reentrancy by potentially malicious code anytime they call a function that could throw an exception?

This is crazy. It seems to me that untrusted code should not be *allowed* to install exception filters in the first place. But the following code runs just fine under the "Internet" permission set:

' VB console application that attacks a C# class library using an exception filter
Imports ClassLibrary
Module Module1
    Sub Main()
        Dim victim As New Victim
        Try
            victim.Update(2, 3)
        Catch When ExploitCode(victim) = True
        End Try
    End Sub
    Function ExploitCode(ByVal victim As Victim) As Boolean
        Console.WriteLine("ExploitCode calling back into victim. IsConsistent returns {0}", victim.IsConsistent)
        Return True
    End Function
End Module

And here is the class under attack (totally benign, but it makes the point). Someone outside this class should never see FALSE returned from IsConsistent. But that's exactly what happens when the VB console app above calls into the code below, because it can reenter the Victim class whenever an exception is thrown (even a handled exception!)

using System;
namespace ClassLibrary {
    public class Victim {
        int a = 0;
        int b = 0;
        int sum = 0;
        public void Update(int newA, int newB) {
            try {
                sum = newA + newB;
                doWork();
                a = newA;
                b = newB;
            }
            finally {
                // ensure state is consistent before we leave
                sum = a + b;
            }
        }
        public bool IsConsistent() {
            return sum == a + b;
        }
        private void doWork() {
            throw new ApplicationException("whatever");
        }
    }
}

The problem here is that the attacker is able to run code between the guarded block and the finally block. The author of this class clearly didn't expect this. One solution is to use a catch block instead of a finally block. That would prevent the attacker's exception filter from running. I wonder if the BCL team thought about this? Time to break out Reflector and search for finally blocks in their code, because if they didn't think about this, there's very likely some vulnerabilities just waiting to be discovered by partially trusted code.