CVE-2024-34695 (C#, published 2024-05-14T15:39Z)
Description: value: WOWS Karma is a reputation system for Wargaming's World of Warships. A user is able to click multiple times on "create" on a post creation prompt before the modal closes, which triggers sending several post creation API requests at once. Due to timing, sending multiple posts simultaneously requests bypasses the cooldown validation, however are not refreshing a user's metrics more than once, due to concurrent karma updates. This issue is fixed in 0.17.4.1.
Severity: nan (CVSS v3 base score: )
Weakness: NVD-CWE-noinfo \u2014 Insufficient Information
There is insufficient information about the issue to classify it; details are unkown or unspecified.
Repository: https://github.com/SakuraIsayeki/WOWS-Karma
Files changed:
WowsKarma.Api/Infrastructure/Attributes/UserAtomicLockAttribute.cs
WowsKarma.Api/Middlewares/UserAtomicLockMiddleware.cs
Commit message: feat(api): Add UserAtomicLock attribute and middleware

This commit adds the `UserAtomicLock` attribute and middleware to the API. The `UserAtomicLock` attribute is used to lock concurrency on a given action to a specific user. The `UserAtomicLockMiddleware` provides the functionality to lock a given action to a specific user. This ensures that only one request from the same user can access the locked action at a time, preventing unwanted concurrent modifications.


Vulnerable code:

Fixed code:
﻿namespace WowsKarma.Api.Infrastructure.Attributes;

/// <summary>
/// Provides an attribute to lock concurrency on a given action to a given user.
/// </summary>
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
public class UserAtomicLockAttribute : Attribute
{
    public UserAtomicLockAttribute() { }
}
﻿using System.Collections.Concurrent;
using WowsKarma.Api.Infrastructure.Attributes;
using WowsKarma.Common;

namespace WowsKarma.Api.Middlewares;

/// <summary>
/// Provides a middleware to lock a given action to a given user.
/// </summary>
public class UserAtomicLockMiddleware : IMiddleware
{
    private static readonly ConcurrentDictionary<Tuple<string, uint>, object> Locks = new();
    private static object ConcurrencyLock = new();

    /// <inheritdoc />
    public async Task InvokeAsync(HttpContext ctx, RequestDelegate next)
    {
        // Get the current user and endpoint
        uint? uid = ctx.User.ToAccountListing()?.Id;
        PathString path = ctx.Request.Path;

        if (uid is null)
        {
            // Pass through.
            await next(ctx);
            return;
        }

        if (ctx.GetEndpoint()?.Metadata.GetMetadata<UserAtomicLockAttribute>() is null)
        {
            // Pass through.
            await next(ctx);
            return;
        }

        bool lockExists;
        // lock (ConcurrencyLock)
        // {
             lockExists = Locks.TryGetValue(new(path, uid.Value), out _) || !Locks.TryAdd(new(path, uid.Value), new());
        // }
        
        // Get or try to add the lock object.
        if (lockExists)
        {
            // Lock is already taken.
            ctx.Response.StatusCode = StatusCodes.Status429TooManyRequests;
            return;
        }
        
        // Hold the lock for the request's duration.
        try
        {
            await next(ctx);
        }
        finally
        {
            // lock (ConcurrencyLock)
            // {
                // Release the lock.
                Locks.TryRemove(new(path, uid.Value), out _);
            // }
        }
    }
}