Avoid spurious crash GetCachedStoreData
What the developer wrote
Authored by Nicolas Dorier
Avoid spurious crash GetCachedStoreData
What changed, and why it matters
This commit swaps a regular dictionary for a thread-safe concurrent dictionary when caching store data attached to a web request. The stated goal is to prevent rare, random server crashes caused by multiple threads accessing the cache at the same time. It is a hardening fix rather than a clear-cut exploitable vulnerability, and the commit message does not frame it as a security issue.
Treat as a stability/reliability improvement. Review whether callers expect overwrite semantics on `AddCachedStoreData`, because `TryAdd` will silently ignore duplicate inserts. Monitor for related crash reports before and after the patch; no immediate security response is indicated by the diff alone.
Security signals we found
Thread-safety hardening of request-scoped cache
Potential crash/DoS condition from concurrent dictionary access
No explicit security framing by vendor
Evidence from the diff
The patch changes AddCachedStoreData and GetCachedStoreData in BTCPayServer/Extensions.cs to use ConcurrentDictionary<string, StoreData> instead of Dictionary<string, StoreData> and replaces the indexer assignment with TryAdd. The original Dictionary is not thread-safe; concurrent reads/writes from request pipeline threads could corrupt internal state and cause exceptions or undefined behavior. The change reduces the chance of race conditions, though it does not eliminate all concurrency concerns (e.g., duplicate adds with TryAdd now silently fail).
Changed components
BTCPayServer/Extensions.csHttpContext item cache for StoreDataInspect captured patch +5 / −4
diff --git a/BTCPayServer/Extensions.cs b/BTCPayServer/Extensions.cs
index bd5e6b1..159bc0a 100644
--- a/BTCPayServer/Extensions.cs
+++ b/BTCPayServer/Extensions.cs
@@ -1,5 +1,6 @@
using System;
+using System.Collections.Concurrent;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Globalization;
@@ -774,18 +775,18 @@ namespace BTCPayServer
public static StoreData AddCachedStoreData(this HttpContext ctx, StoreData storeData)
{
if (!ctx.Items.TryGetValue("BTCPAY.CACHEDSTOREDATA", out var item) ||
- item is not Dictionary<string, StoreData> dictionary)
+ item is not ConcurrentDictionary<string, StoreData> dictionary)
{
- dictionary = new Dictionary<string, StoreData>();
+ dictionary = new ConcurrentDictionary<string, StoreData>();
ctx.Items["BTCPAY.CACHEDSTOREDATA"] = dictionary;
}
- dictionary[storeData.Id] = storeData;
+ dictionary.TryAdd(storeData.Id, storeData);
return storeData;
}
public static StoreData? GetCachedStoreData(this HttpContext ctx, string storeId)
{
if (!ctx.Items.TryGetValue("BTCPAY.CACHEDSTOREDATA", out var item) ||
- item is not Dictionary<string, StoreData> dictionary)
+ item is not ConcurrentDictionary<string, StoreData> dictionary)
return null;
dictionary.TryGetValue(storeId, out var storeData);
return storeData;
Why this scored 35/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.