0

Good day, we deployed our app that has payment on it with a wallet system. We tried as much as possible to follow every security rule from server to code design. But yesterday we experienced a bridge with javascript logic that made us temporary bring down the backend.

We are running react native on the front end and nodejs on the backend. To make a purchased, we check your returned wallet balance and the inputted amount to see if you have sufficient fund to perform the operation. Example

if(store.userBal >= parseFloat(formData.amount)){
// carry on
}else{
alert("insufficient fund")
}

Then also on the backend we have something similar

exports.performTransaction = (req, res) =>{
const getBal = await WalletBal.findOne({ where: { id: req.user.id } });

if(getBal.availabelBal >= parseFloat(req.body.amount){
//make api call
// if successful
// decrement balance by amount

//Other updates and send notification
}
}

But some reason we discovered a bridge was able to bypass the logic both on front end and backend and perform transactions into a negative balance. We have been making updates and tightening our verification process but still not confident of the root cause of this. Please some insight will be very helpful

1
  • 1
    What type is getBal.availabelBal at runtime? Please edit your question to also include the code where you actually use req.body.amount after the check passes, since that will be important too.
    – Polynomial
    Oct 28, 2022 at 19:10

1 Answer 1

0

A negative amount entered into req.body.amount will pass the check you have implemented, because it is always smaller than the user's balance. My guess is that you later subtract the requested amount from their wallet. If the subtracted value is negative, it adds money to the wallet instead of removing it, and the user gets free money. You need to verify that the requested amount is a positive number.

Your existing code is written in a way that invites vulnerabilities that arise from parsing ambiguity. For example, let's say you write this code:

if (getBal.availableBal >= parseFloat(req.body.amount))
{
    const result = await WalletBal.withdraw(req.body.amount);
    // ...
}

Now imagine the WalletBal.withdraw() function looks like this:

withdraw = function(requestedAmount)
{
    const amount = Number(requestedAmount);
    doBankTransfer(this.account, requestedAmount);
    this.availableBal -= amount;
    return true;
}

The user input is now being parsed twice, once by parseFloat() for the check and again by the Number() constructor for the actual operation. There are behavioural differences between these functions, which an attacker can use to bypass the check.

For example, look what happens if I enter "0xffff" as the amount to withdraw:

> parseFloat("0xffff")
0
> Number("0xffff")
1048575

This bypasses the balance availability check, because the first result is zero, but causes an amount of 1048575 to be withdrawn.

In addition, using floating point numbers to represent currency is fraught. You need to deal with precision and representation issues, as well as odd values like NaN, -NaN, Infinity, -Infinity, and -0. If you must use floats, make sure you verify that the parsed result is a normal number using IsNaN() and IsFinite().

Another problem with your code is that you're doing the checks on a request-by-request basis. Instead, your request handler code should validate the input string (e.g. with a regex such as /^[0-9]+([.][0-9]+)?$/.test(str)), then convert the user's input string to a number.

You should then have a withdrawal function (or similar) for performing the actual balance change, which checks that the input to the function is a positive finite number that is less than or equal to the balance. For example, a wallet class might have this function:

this.withdraw = function(requestedAmount)
{
    if ((typeof requestedAmount !== 'number') ||
        isNaN(requestedAmount) ||
        !isFinite(requestedAmount))
    {
        return false; // improper input value
    }
    if (requestedAmount > this.availableBal)
    {
        return false; // insufficient funds
    }
    // do the transfer...
}

This ensures that there is a common check for numeric validity and sufficient balance, rather than having to implement the same checks over and over again for each request type, which is prone to errors and mistakes.

Another problem you might run into here is floating point representation. For example, if my balance is 5.32 and I ask to withdraw 0.01, I don't end up with 5.31 left in my account:

> 5.32 - parseFloat("0.01")
5.3100000000000005

This is because JS uses IEEE 754 64-bit double precision floating point numbers internally, and not all numbers can be represented. This can result in weird and erroneous behaviour.

For subtraction this does not cause a huge amount of error, but if you end up doing any multiplication or division it can scale up the error quite a bit. The small errors can also cause problems when you try to compare numbers.

I would recommend using a currency type library to handle these numbers properly, to save you from these representation issues.

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .