0

During a Security review at the company I work for, an issue was raised with a Web.API returning too much information if the user were to change for the example the order by value that is provided.

Example of a legitimate call: https://mysite/controller/myData?$orderby=realcolumn

Example of a malicious call: https://mysite/controller/myData?$orderby=fakecolumn

In the second case, the api is returning:

{
  "$id": "1",
  "$type": "System.Web.Http.HttpError, System.Web.Http",
  "Message": "The query specified in the URI is not valid. Could not find a property named 'fakecolumn' on type 'MyObject.Models.MyData."
}

While I do not see this as being a large security concern and as a developer having this type of response is helpful... I am being asked to make this more generic – basically provide as little information as possible.

I don’t know how to trap this prior to sending the response back to the user. If I walk through the code and stop prior to returning the result, the data is there. In this case the order by doesn’t appear to be evaluated until after the return occurs in the controller. Is there a way to perform the evaluation at the server, trap for errors like this and return a more generic response?

Code snippets below, I appreciate any assistance provided.

From the Controller

RepositoryMyData _repo;

[HttpGet]
public IQueryable<myData> myData()
{
   return _repo.myData();
}

From the Repository class – using Entity Framework…

public DbQuery<myData> myData()
{
   return (DbQuery<myData>)_contextProvider.Context.myData
}
Chris C
  • 1
  • 2
  • 2
    Hollywood movies in this case usually display a flashing red **ERROR** in a thick red border. Just sayin'. – Ondrej Tucny Dec 08 '15 at 23:08
  • You should probably add a custom exception handler which ensures the error response doesn't contain the "too much info". Create a subclass of ExceptionFilterAttribute and add it to the global filters. See: http://stackoverflow.com/questions/16243021/return-custom-error-objects-in-web-api – Mark Dec 08 '15 at 23:15
  • Mark - Thank you for the feedback! I was able to get a custom error handler working that works for exceptions in the controller. However still if I pass in https://mysite/controller/myData?$orderby=fakecolumn or $top=ten for example... I still get the default JSON error back as shown in my post. It is as if the $orderby / $top, etc... isn't evaluated inside the controller or happens after and the custom handler doesn't pick it up. – Chris C Dec 09 '15 at 18:00
  • Are you also going to close down /$metadata somehow? This will be the 'information disclosure' biggest gap. Not sure i get the 'information disclosure' weight balance of 'column does not exist' versus the "here's the complete schema for it"? – Marvin Smit Dec 10 '15 at 09:45

1 Answers1

0

May be something like this would help? -

Option 1:

//customer exception class
public class QueryError : Exception{
}

[HttpGet]
public IQueryable<myData> myData()
{
   try{
        return _repo.myData();
   }catch(Exception e){
        throw new QueryError("An error occurred.");
        // not a good practice though, too much exceptions to handle for the system.
   }
}

Option 2:.

public  class CustomHandleErrorAttribute : System.Web.Mvc.HandleErrorAttribute
{
    public override void OnException(ExceptionContext context)
    {
        context.ExceptionHandled = true;
        context.HttpContext.Response.Clear();
        context.Result = //build the result with needed information
    }
}

[CustomHandleError]
public lass <Controller>.......
brainless coder
  • 6,310
  • 1
  • 20
  • 36
  • Thank you for the feedback! Option 2 works if an error occurs in the controller code. For example if I throw an exception like: throw new Exception("Something bad happened"); return _repo.myData(); Then the custom handler picks it up. However if an error is generated through passing bad info through the url, for example $top=ten or $orderby=invalidcolumn.... then the custom handler doesn't get called and the default exception is generated as shown in the JSON in my original post. – Chris C Dec 09 '15 at 17:54