ricardocarva / MortiSmart

https://mortismart.netlify.app/
Apache License 2.0
2 stars 4 forks source link

[Security Hotspots] Make sure the regex used here, which is vulnerable to super-linear runtime due to backtracking, cannot lead to denial of service. #32

Closed ricardocarva closed 9 months ago

ricardocarva commented 9 months ago

On file public/scripts/components/InputForm.js

const formatNumber = (n) => { // format number 1000000 to 1,234,567 return n.replace(/\D/g, "").replace(/\B(?=(\d{3})+(?!\d))/g, ","); };

Feedback: Make sure the regex used here, which is vulnerable to super-linear runtime due to backtracking, cannot lead to denial of service.

More info on: https://sonarcloud.io/project/security_hotspots?id=ricardocarva_MortiSmart&hotspots=AYw3YOHoKC8qIV4UHP-Z

ricardocarva commented 9 months ago

The regex looks fine to me. It looks like we can limit the length of input that the regex processes. This would help mitigate the potential for exponential backtracking. I don't think anything is going to happen though, but it wouldnt hurt to limit to a certain number of characters like 15 digits. I cannot think of a property that costs more than that to our users.

Perhaps we could do:

const MAX_ALLOWED_LENGTH = 15; // Set the desired maximum length

const formatNumber = (n) => {

  // Check if the input length exceeds the allowed limit
  if (n.length > MAX_ALLOWED_LENGTH) {
  // Handle the case where input length exceeds the limit
    return "Input length exceeds the allowed limit";
  }

  else {
  return n.replace(/\D/g, "").replace(/\B(?=(\d{3})+(?!\d))/g, ",");
  }
}
ricardocarva commented 9 months ago

Addressed on #38