UsePAY

The `createTicket` function calls the constructor in `TicketPack` along with a `msg.value`. `TicketPack` also has a `fallback` function that implements a proxy to a commander address. The `fallback` function (and therefore the proxy) is never called even when `msg.value` is sent to the contract because `TicketPack` has a receive function which takes in the `msg.value` instead. After this happens, tokens are locked in the `TicketPack` contract as there isn't any withdraw functionality either. The solution to this particular problem would be to remove the `receive` function.

In the function `calculate` there is this logic: ```solidity uint quantityCount = packInfo.total - quantity - totalUsedCount; uint qunaityValue = _percentValue(packInfo.price,packInfo.noshowValue) * quantityCount; _transfer(packInfo.tokenType,owner,qunaityValue); ``` `qunaityValue` is transferred to the owners address (withdrawing their earnings). `qunaityValue` is dependant on `quantityCount`. The problem with `quantityCount` is that totalUsedCount can be artificially increased such that `quantityCount` overflows (due to the solidity version being `>= 0.7.0`) or `quantityCount` can simply be set to 0 in version `0.8.0` and above. Here is the attack flow: For this example let's assume there is 100 available packs 1) The attacker buys 100 packs, as a result `quantity` decreases by 100. 2) The attacker requests a refund for these packs before `packInfo.times1`, this way `quantity` increases by 100 (cancelling out the previous decrease) and `totalUsedCount` increases by 100. After the attack, assuming regular users buy the tickets and eventually the time comes where the owner would like to withdraw his earnings, all the funds will be locked away from them as: ```solidity uint quantityCount = packInfo.total - quantity - totalUsedCount; ``` Since `totalUsedCount` was artificially inflated, `quantityCount` will have underflowed (in versions `^0.7.0`) making `quantityCount` so large that the transfer will always fail due to insufficient funds in the contract. Or in versions `^0.8.0`: since `totalUsedCount` is so big, the calculation will fail and the owner will never be able to withdraw is funds.

The Checks-Effects-Interactions pattern is not present in the code base hence re-entrancy attacks are present. Attack scenario for BNB/ETH/KLA: 1) Attack buys a pack and their `hasCount` increases, allowing them to pass the `canUse` check in `requestRefund` 2) When the attacker calls `requestRefund`, eventually BNB/ETH will be sent to the attackers address at the value of `packInfo.price` 3) In the attackers address there is a fall back function which calls `requestRefund` again. It passes the `canUse` check because the attackers `hasCount` has not yet been reduced. Therefore the attacker receives another refund. 4) The cycle continues until the contract is drained of tokens.

The owner of a subscription pack is always vulnerable to a grief attack depending on the value of `noshowLimit`. For example, an `owner` sets `packInfo.total` as 40 and `noshowLimit` as 15. Lets say 25 packs are bought by regular customers and an attacker buys 15 subscription packs. The attacker can refund his packs immediately after `packInfo.times2` is reached, setting `isLive` to 1. When time comes for the `owner` to withdraw his earnings from the other 25 packs, they are unable to do so due to the `checkLive` check on the `calculate` function, losing out on all his earned tokens.

The contracts `BSC_SubscriptionCreator` and `BSC_TicketCreator` have essentially identical logic for the swap upon creating a pack. However in `BSC_SubscriptionCreator`, `tokenIdx` defaults to: ```solidity uint16[2] private tokenIdx = [101,550]; ``` and in `BSC_TicketCreator`, `tokenIdx` defaults to: ```solidity uint16[2] private tokenIdx = [550,101]; ``` Which are in the opposite order to each other, indicating one of the swaps being in the wrong order. The same problem extends to `couponCreator` contract but the contract does not include a `_swap` function.

In the `noShowRefund`, a percentage is taken off the refund value twice, once here: ```solidity uint refundValue = _percentValue(packInfo.price,95); ``` Which makes the refund value 95% of the price. This value is further altered here: ```solidity for( uint i = 0; i < _addrList.length; i++ ) { address _to = _addrList[i]; buyList[_to].hasCount--; (values[i], swaps[i]) = _refund(_to,refundValue,10); } ``` Where the refund value is further calculated against for fees and the total to be returned to the customer is 0.90 * 0.95 * `packInfo.price`.

`BSC_CouponCreator` features a lot of functionality that facilitates a swap, such as `tokenIdx`, `swapCount` and functions such as `setTokenIdx` and `setSwapCount`. However a swap never takes place in the contract.

It is unclear why in `BSC_Commander.sol`, the `checkFee` function has a require statement that: ```solidity require( msg.value > getPrice() * (n) , "C01"); ``` While in the sister contracts in `Commander.sol` and `KLA_Commander.sol`, the `n` variable is hard coded to 5. ```solidity require( msg.value > getPrice() * n * 5 , "C01"); ``` This inconsistency is also found in the `getCountFee` function.

There is some loss in accuracy when calculating percentages at the upper bound of a percentage change. For example 60 out of 100 is calculated as 59%. 500 out of 1000 is calculated as 49%, and so on.

In `launchSetAddresses()` and `launchCalculate()` a check is passed only if `multiSign.count == numConfirmationsRequired`. However, it should be allowed to pass if `multiSign.count >= numConfirmationsRequired` to make it consistent with `confirmSetAddresses()` and `confirmCalculate()`.