Could a security breach happen through Reflection?

Question

Assume you have a function in C# which allows you to print out the properties of a model like this:

public void Export<T>(List<T> list)
{    
    // get properties of Model
    Type modelType = typeof(T);
    var modelInstance = Activator.CreateInstance(modelType);
    PropertyInfo[] properties = modelInstance.GetType().GetProperties();

    foreach (var property in properties)
    {
        if (property.CanRead)
            PRINTOUT(property.Name);
    }

    foreach (var entity in list)
    {
        foreach (var property in properties)
        {
            if (property.CanRead)
                PRINTOUT(property.GetValue(entity, null));
        }
    }
}

Now, this function is in a controller so, as I've read, is a publicly exposed HTTP endpoint! Is it possible for a malicious user to ask to PRINTOUT properties he should not see?

To avoid this, I'd like to call this function from another controller which pass in a very specific type.

For example, PrintoutCustomers in the controller Customers calls that function while also the controller Products (with the action PrintoutProducts) can call the same exact function but pass in a list of products (instead of customers).

The user will then have a list of specifics publicly accessible HTTP endpoints and no one of them will allows him to do anything harmful.

I'd like that function to be private and shared between all my controllers: is it possible? Can I do that? Or do I really have to copypaste the same method everywhere I need it (and wasted my time tried to make it generic using typeof and reflections)?

Answer 1

Why would you put that method in a controller? It belongs into a static helper class or something like this. You could even make it an extension method, if you want to go fancy.

public static class OutputExtensions
{
    public static void Export<T>(this List<T> list)
    {
        // get properties of Model
        PropertyInfo[] properties = typeof(T).GetProperties();

        foreach (var property in properties)
        {
            if (property.CanRead)
                PRINTOUT(property.Name);
        }

        foreach (var entity in list)
        {
            foreach (var property in properties)
            {
                if (property.CanRead)
                    PRINTOUT(property.GetValue(entity, null));
            }
        }
    }
}

(This version also contains the fix suggested by Trevor Pilley in his comment)

You could use it like this in every controller:

listOfModels.Export();

Answer 2

In this case, I think Shannon's maxim applies:

The enemy knows the system.

This is normally cited in the context of cryptographic algorithms, but it works here too. Yes, this method provides malicious users with lists of stuff they might not otherwise know about, but in theory, they could also just query your server for all possible combinations of words and see which ones work; that would also give them a list of open endpoints.

When you leave your house, you don't put camouflage over your door - you lock it. The same principle works here: if there's something a user shouldn't be able to do, make sure your authentication and authorisation code works, and properly denies access to anything you want to keep them out of. Simply hiding things is known as security through obscurity, and in short, it doesn't work.

When you make software that can connect to the web, it's ok (and often a good idea) to hide certain things to reduce the attention they receive from attackers, but even then you should assume that at some point a malicious user will find out that they exist, and you should secure them against malicious use.

In your particular case, I'd say the following:

• Can you do without this particular functionality?
• Can you do it another way that exposes less of your system?
• Can a malicious user use this function to do something you don't want them to be able to do?

If the answer to any of these is "yes", then you should probably not do this. If and only if all of them are "no", then you're fine. Personally, I think you're forgetting a simple rule: if it doesn't need to be accessible from the web, don't put it in a controller! Consider moving this method to an internal class in your logic layer, somewhere that only your trusted code can get at it.