I have an ASP.NET Core 6.0 Web API application with the below ExceptionHandlerMiddleware:
public class ExceptionHandlerMiddleware
{
private readonly RequestDelegate next;
public ExceptionHandlerMiddleware(RequestDelegate next)
{
this.next = next;
}
public async Task InvokeAsync(HttpContext context)
{
try
{
await this.next(context).ConfigureAwait(false);
}
catch (Exception ex)
{
await this.ConvertExceptionAsync(context, ex).ConfigureAwait(false);
}
}
private Task ConvertExceptionAsync(HttpContext context, Exception exception)
{
HttpStatusCode httpStatusCode = HttpStatusCode.InternalServerError;
context.Response.ContentType = "application/json";
var errorResponse = new ErrorResponse();
var result = string.Empty;
switch (exception)
{
case ValidationException validationException:
httpStatusCode = HttpStatusCode.BadRequest;
errorResponse.Error.Errors = JsonSerializer.Serialize(validationException.ValidationErrors);
break;
case BadRequestException badRequestException:
httpStatusCode = HttpStatusCode.BadRequest;
errorResponse.Error.Message = badRequestException.Message;
break;
case NotFoundException notFoundException:
httpStatusCode = HttpStatusCode.NotFound;
break;
case NotAcceptableException notAcceptableException:
httpStatusCode = HttpStatusCode.NotAcceptable;
errorResponse.Error.Message = notAcceptableException.Message;
break;
case Exception ex:
httpStatusCode = HttpStatusCode.BadRequest;
break;
}
errorResponse.ApiVersion = context.GetRequestedApiVersion()?.ToString();
errorResponse.Error.Code = (int)httpStatusCode;
context.Response.StatusCode = (int)httpStatusCode;
if (string.IsNullOrEmpty(result))
{
errorResponse.Error.Message = exception.Message;
}
var serializeOptions = new JsonSerializerOptions
{
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
WriteIndented = true,
};
result = JsonSerializer.Serialize(errorResponse, serializeOptions);
return context.Response.WriteAsync(result);
}
}
It returns the error in the custom format like mentioned below
{
"error": {
"code": 404,
"message": "Education (a65f7f0c-2a29-4da0-bd4b-d737320730c6) is not found",
"errors": "[]"
},
"apiVersion": "1.0"
}
During the security scan, it is reported that
The file handles an Exception or runtime Error ex. During the exception handling code, the application exposes the exception details to WriteAsync, in method ConvertExceptionAsync
Avoid information leak through error messages
- Don’t expose users exception output data
- Properly handle exception for each methods
- Configure a global handler to prevent unhandled errors
Should I ask the security team to suppress this issue? Or this is something that can be addressed at the application level?
Update: As I just want to address the reported issue, "context.Response.WriteAsync" is moved to finally block. I hope it should address the above security issue.
public async Task InvokeAsync(HttpContext context)
{
bool isExceptionReported = false;
string result = string.Empty;
try
{
await this.next(context).ConfigureAwait(false);
}
catch (Exception ex)
{
isExceptionReported = true;
result = this.ConvertException(context, ex);
}
finally
{
if (isExceptionReported)
{
await context.Response.WriteAsync(result).ConfigureAwait(false);
}
}
}
private string ConvertException(HttpContext context, Exception exception)
{
HttpStatusCode httpStatusCode = HttpStatusCode.InternalServerError;
context.Response.ContentType = "application/json";
var errorResponse = new ErrorResponse();
var result = string.Empty;
switch (exception)
{
case ValidationException validationException:
httpStatusCode = HttpStatusCode.BadRequest;
errorResponse.Error.Errors = JsonSerializer.Serialize(validationException.ValidationErrors);
break;
case BadRequestException badRequestException:
httpStatusCode = HttpStatusCode.BadRequest;
errorResponse.Error.Message = badRequestException.Message;
break;
case NotFoundException notFoundException:
httpStatusCode = HttpStatusCode.NotFound;
break;
case NotAcceptableException notAcceptableException:
httpStatusCode = HttpStatusCode.NotAcceptable;
errorResponse.Error.Message = notAcceptableException.Message;
break;
case Exception ex:
httpStatusCode = HttpStatusCode.BadRequest;
break;
}
errorResponse.ApiVersion = context.GetRequestedApiVersion()?.ToString();
errorResponse.Error.Code = (int)httpStatusCode;
context.Response.StatusCode = (int)httpStatusCode;
if (string.IsNullOrEmpty(result))
{
errorResponse.Error.Message = exception.Message;
}
var serializeOptions = new JsonSerializerOptions
{
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
WriteIndented = true,
};
result = JsonSerializer.Serialize(errorResponse, serializeOptions);
return result;
}
Here is now I handle this "Exception to Http-Status-Code" tension.
[ExcludeFromCodeCoverage, Serializable]
public class BusinessLayerFinalException : Exception
{
public const int PreferredHttpStatusCodeDefault = 500;
public const bool ShowInnerExceptionToCustomerDefault = false;
public const string
CustomerFacingMessageDefault = "There was an error with your request";
public int? PreferredHttpStatusCode { get; set; } = PreferredHttpStatusCodeDefault;
public bool ShowInnerExceptionToCustomer { get; set; } = ShowInnerExceptionToCustomerDefault;
public string CustomerFacingMessage { get; set; } = CustomerFacingMessageDefault;
public BusinessLayerFinalException()
{
}
public BusinessLayerFinalException(string message)
: base(message)
{
}
public BusinessLayerFinalException(string message, Exception inner)
: base(message, inner)
{
}
protected BusinessLayerFinalException(
SerializationInfo info,
StreamingContext context) : base(info, context)
{
if (null != info)
{
this.PreferredHttpStatusCode = info.GetInt32(nameof(this.PreferredHttpStatusCode));
this.ShowInnerExceptionToCustomer = info.GetBoolean(nameof(this.ShowInnerExceptionToCustomer));
this.CustomerFacingMessage = info.GetString(nameof(this.CustomerFacingMessage));
}
}
public override void GetObjectData(
SerializationInfo info,
StreamingContext context)
{
base.GetObjectData(info, context);
if (null != info)
{
info.AddValue(nameof(this.PreferredHttpStatusCode), this.PreferredHttpStatusCode);
info.AddValue(nameof(this.ShowInnerExceptionToCustomer), this.ShowInnerExceptionToCustomer);
info.AddValue(nameof(this.CustomerFacingMessage), this.CustomerFacingMessage);
}
}
public BusinessLayerFinalException(
string message,
int preferredHttpStatusCode,
bool showInnerExceptionToCustomer,
string customerFacingMessage) : base(message)
{
this.PreferredHttpStatusCode = preferredHttpStatusCode;
this.ShowInnerExceptionToCustomer = showInnerExceptionToCustomer;
this.CustomerFacingMessage = customerFacingMessage;
}
public BusinessLayerFinalException(
string message,
Exception innerException,
int preferredHttpStatusCode,
bool showInnerExceptionToCustomer,
string customerFacingMessage) : base(message, innerException)
{
this.PreferredHttpStatusCode = preferredHttpStatusCode;
this.ShowInnerExceptionToCustomer = showInnerExceptionToCustomer;
this.CustomerFacingMessage = customerFacingMessage;
}
}
Then I write 2 (and only 2) Middleware.
One to handle "Exception" (where I give a 500 and NO extra context information.
And one more to handle "BusinessLayerFinalException", which (intelligently?? :) ) looks at BusinessLayerFinalException and figures out what to show.
I'll show the more interesting one below:
/// <summary>
/// Middleware to handle a specific exception 'BusinessLayerFinalException'. Loosely based on : https://learn.microsoft.com/en-us/aspnet/core/web-api/handle-errors?view=aspnetcore-5.0#use-exceptions-to-modify-the-response
/// </summary>
public class BusinessLayerFinalExceptionFilter : IActionFilter, IOrderedFilter
{
public int Order { get; } = int.MaxValue - 10;
public const string FullErrorMessageTemplate = "({0}), ({1})";
public void OnActionExecuting(ActionExecutingContext context)
{
/* as per the microsoft example, this method is currently empty */
}
public void OnActionExecuted(ActionExecutedContext context)
{
if (context.Exception is BusinessLayerFinalException exception)
{
string errorMsg = exception.ShowInnerExceptionToCustomer ? string.Format(FullErrorMessageTemplate, exception.CustomerFacingMessage, ExceptionExtensions.GenerateFullFlatMessage(exception, false)) : exception.CustomerFacingMessage;
context.Result = new ObjectResult(errorMsg)
{
StatusCode = exception.PreferredHttpStatusCode ?? (int)HttpStatusCode.InternalServerError
};
context.ExceptionHandled = true;
}
}
}
ExceptionExtensions.GenerateFullFlatMessage
(this loops over all .InnerException(s) to provide a full stack trace.
..
You could also inject a "IsDevelopment" (https://learn.microsoft.com/en-us/aspnet/core/fundamentals/environments?view=aspnetcore-6.0 ) boolean to always show the super-details in that environment.
The BusinessLogic then has the "responsibility" to always throw a BusinessLayerFinalException...or anything else will be caught with the "Exception" middleware and give a no-information 500 to the outside world.
The benefit of BusinessLayerFinalException (IMHO) is that I am allowing the business logic to give the top-rest layer the "hints" that it needs ... WITHOUT breaking the best practice seen below.
=====
https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/exception-throwing
❌ DO NOT return error codes. Exceptions are the primary means of reporting errors in frameworks.
✔️ DO report execution failures by throwing exceptions.
=====
I have seen (too many) code bases that started going back to VB6 mentality and returning error-codes all over the place....with http numbers. A complete hijack of the OO language.