Fix possible race condition when updating a PaymentRequest (#7281)
What the developer wrote
Authored by Nicolas Dorier
Fix possible race condition when updating a PaymentRequest (#7281)
What changed, and why it matters
This commit fixes a potential race condition in how BTCPay Server updates the status of a Payment Request. Previously, the code read the record, changed its status in memory, and saved it back. If two processes did this at the same time, one update could overwrite the other, possibly causing duplicate or missed events. The fix uses a single SQL UPDATE statement that only changes the status if it is different, then checks the result before publishing an event. The commit message calls this a 'possible race condition' but does not describe a specific security outcome.
Treat as a hardening/defensive fix. Review other repositories using similar read-modify-save status-update patterns and consider applying the same atomic-update pattern. No emergency response is indicated by the supplied materials.
Security signals we found
Race condition in status update logic
Read-modify-write pattern replaced with atomic conditional UPDATE
Event publication now gated by affected-row count and re-read verification
No explicit security claim or CVE in commit or supplied references
Evidence from the diff
PaymentRequestRepository.UpdatePaymentRequestStatus was converted from an EF Core read-modify-save pattern to a direct Dapper UPDATE query with a conditional WHERE clause (‘Status’ != @status). After the update, it re-reads the entity and verifies the status before publishing a PaymentRequestEvent. The unit test was adjusted to wait for the PaymentRequestEvent after editing a payment request. The change reduces the chance of lost updates and duplicate event publication under concurrency, but the commit does not state that an exploitable vulnerability exists.
Changed components
BTCPayServer.Services.PaymentRequests.PaymentRequestRepositoryBTCPayServer.Tests.PaymentRequestTestsInspect captured patch +19 / −10
diff --git a/BTCPayServer.Tests/PaymentRequestTests.cs b/BTCPayServer.Tests/PaymentRequestTests.cs
index 05e12b4..c08dbbe 100644
--- a/BTCPayServer.Tests/PaymentRequestTests.cs
+++ b/BTCPayServer.Tests/PaymentRequestTests.cs
@@ -386,9 +386,9 @@ namespace BTCPayServer.Tests
request.ExpiryDate = null;
var paymentRequest = await repo.FindPaymentRequest(prId, null);
paymentRequestController.HttpContext.SetPaymentRequestData(paymentRequest);
- paymentRequestController.EditPaymentRequest(prId, request).Result
+ Assert.Equal(prId, paymentRequestController.EditPaymentRequest(prId, request).Result
.AssertType<RedirectToActionResult>()
- .RouteValues.Last().Value.ToString();
+ .RouteValues!.Last().Value.ToString());
paymentRequestController.HttpContext.SetPaymentRequestData(null);
request = new UpdatePaymentRequestViewModel()
{
@@ -400,9 +400,12 @@ namespace BTCPayServer.Tests
Description = "description"
};
- prId = paymentRequestController.EditPaymentRequest(null, request).Result
- .AssertType<RedirectToActionResult>()
- .RouteValues.Last().Value.ToString();
+ await tester.WaitForEvent<PaymentRequestEvent>(async () =>
+ {
+ prId = (await paymentRequestController.EditPaymentRequest(null, request))
+ .AssertType<RedirectToActionResult>()
+ .RouteValues!.Last().Value.ToString();
+ }, ev => ev.Data.Status == PaymentRequestStatus.Expired);
(await paymentRequestController.PayPaymentRequest(prId, false)).AssertType<BadRequestObjectResult>();
}
diff --git a/BTCPayServer/Services/PaymentRequests/PaymentRequestRepository.cs b/BTCPayServer/Services/PaymentRequests/PaymentRequestRepository.cs
index abf1867..3a80696 100644
--- a/BTCPayServer/Services/PaymentRequests/PaymentRequestRepository.cs
+++ b/BTCPayServer/Services/PaymentRequests/PaymentRequestRepository.cs
@@ -6,6 +6,7 @@ using System.Threading.Tasks;
using BTCPayServer.Client.Models;
using BTCPayServer.Data;
using BTCPayServer.Services.Invoices;
+using Dapper;
using Microsoft.EntityFrameworkCore;
namespace BTCPayServer.Services.PaymentRequests
@@ -100,13 +101,18 @@ namespace BTCPayServer.Services.PaymentRequests
public async Task UpdatePaymentRequestStatus(string paymentRequestId, Client.Models.PaymentRequestStatus status, CancellationToken cancellationToken = default)
{
await using var context = _ContextFactory.CreateContext();
- var paymentRequestData = await context.FindAsync<PaymentRequestData>(paymentRequestId);
- if (paymentRequestData == null || paymentRequestData.Status == status)
+ var conn = context.Database.GetDbConnection();
+ var affectedRows = await conn.ExecuteAsync("""
+ UPDATE "PaymentRequests"
+ SET "Status" = @status
+ WHERE "Id" = @id AND "Status" != @status;
+ """, new{ id = paymentRequestId, status = status.ToString()});
+ if (affectedRows == 0)
return;
- paymentRequestData.Status = status;
-
- await context.SaveChangesAsync(cancellationToken);
+ var paymentRequestData = await context.FindAsync<PaymentRequestData>(paymentRequestId);
+ if (status != paymentRequestData?.Status)
+ return;
_eventAggregator.Publish(new PaymentRequestEvent()
{
Data = paymentRequestData,
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.