Need

Ensure protection against accidental deletion of critical objects.

Context

• Usage of C# for building robust and scalable applications
• Usage of Microsoft.AspNetCore.Mvc for building web applications with ASP.NET Core
• Usage of System.Linq for querying and manipulating data in .NET applications
• Usage of MyWebApplication.Data for accessing and manipulating data in a web application
• Usage of MyWebApplication.Models for defining and managing models in a web application

Description

Non compliant code

        using Microsoft.AspNetCore.Mvc;
using System.Linq;
using MyWebApplication.Data;
using MyWebApplication.Models;

namespace MyWebApplication.Controllers
{
    [Route("api/[controller]")]
    [ApiController]
    public class RecordsController : ControllerBase
    {
        private readonly ApplicationDbContext _context;

        public RecordsController(ApplicationDbContext context)
        {
            _context = context;
        }

        // DELETE: api/Records/5
        [HttpDelete("{id}")]
        public ActionResult DeleteRecord(int id)
        {
            var record = _context.Records.Find(id);
            if (record == null)
            {
                return NotFound();
            }

            _context.Records.Remove(record);
            _context.SaveChanges();

            return record;
        }
    }
}
        
        

This C# code snippet is a controller class RecordsController for an ASP.NET Core application. This class has a DeleteRecord method that deletes a record from the database based on the provided id.

The DeleteRecord method first finds the record in the database using the Find method. If the record is not found, it returns a 404 Not Found response. If the record is found, it removes the record from the database using the Remove method and then saves the changes to the database using the SaveChanges method.

The vulnerability lies in the fact that there are no checks or protections against deleting critical records. Any user who can access this API endpoint and knows the id of a record can delete it. This could lead to accidental or malicious deletion of critical data.

Steps

• Implement authentication and authorization mechanisms to ensure that only authorized users can delete records.
• Add validation checks to ensure that the record being deleted is not a critical object that should not be deleted.
• Consider implementing a soft delete mechanism instead of permanently deleting records.
• Implement logging and auditing mechanisms to track record deletions and identify any accidental deletions.
• Perform thorough testing to ensure the fix is effective and does not introduce any new vulnerabilities.

Compliant code

        using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using System.Linq;
using MyWebApplication.Data;
using MyWebApplication.Models;

namespace MyWebApplication.Controllers
{
    [Route("api/[controller]")]
    [ApiController]
    public class RecordsController : ControllerBase
    {
        private readonly ApplicationDbContext _context;

        public RecordsController(ApplicationDbContext context)
        {
            _context = context;
        }

        // DELETE: api/Records/5
        [Authorize(Roles = "Admin")] // Only users with Admin role can delete records
        [HttpDelete("{id}")]
        public ActionResult DeleteRecord(int id)
        {
            var record = _context.Records.Find(id);
            if (record == null)
            {
                return NotFound();
            }

            // Check if the record is a critical object
            if (record.IsCritical)
            {
                return BadRequest("Critical records cannot be deleted.");
            }

            // Implement soft delete
            record.IsDeleted = true;
            _context.SaveChanges();

            // Log the deletion
            _context.Logs.Add(new Log 
            { 
                Action = "Delete", 
                RecordId = id, 
                UserId = User.Identity.Name, 
                Timestamp = DateTime.Now 
            });
            _context.SaveChanges();

            return record;
        }
    }
}
        
        

The updated code includes several changes to address the vulnerability:

1. Authentication and Authorization: The [Authorize(Roles = "Admin")] attribute is added to the DeleteRecord method. This ensures that only users with the "Admin" role can delete records.

2. Validation Checks: Before deleting a record, the code checks if the record is a critical object by checking the IsCritical property of the record. If the record is critical, the method returns a bad request response.

3. Soft Delete: Instead of permanently deleting the record from the database, the code sets the IsDeleted property of the record to true. This is a soft delete mechanism that allows the record to be recovered if necessary.

4. Logging and Auditing: The code logs the deletion action, including the ID of the deleted record, the user who deleted the record, and the timestamp of the deletion. This information can be used to track record deletions and identify any accidental deletions.

5. Testing: After implementing these changes, thorough testing should be performed to ensure the fix is effective and does not introduce any new vulnerabilities.

References

• 101. Lack of protection against deletion