Ebisu’s Bay

Violation of check-effect-interaction patterns could lead to a reentrancy attack vector. A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. In the current contract, functions `makeListing()` and `makePurchase()` will invoke functions implemented from external injected NFT tokens (e.g., `_nft.safeTransferFrom()`). The status of the listing nft is updated after those external invocations. Thus, introducing attack vectors for reentrancy. For example, ```solidity=186 if(listing.is1155){ IERC1155(listing.nft).safeTransferFrom(listing.seller, msg.sender, listing.nftId, 1, ""); }else { IERC721(listing.nft).safeTransferFrom(listing.seller, msg.sender, listing.nftId); } _asyncTransfer(listing.seller, listing.price - listing.fee - listing.royalty); address ipHolder = royalties[listing.nft].ipHolder; if(ipHolder != address(0)){ _asyncTransfer(ipHolder, listing.royalty); } activeListings.remove(activeListings.keyForId(_id)); listing.purchaser = msg.sender; listing.saleTime = block.timestamp; completeListings.set(keccak256(abi.encodePacked(_id)), listing); ``` The code snippets above are from `makePurchase()`, where `safeTransferFrom()` is invoked before the order is set as "complete". The `safeTransferFrom()` invocation is from external source and cannot be trusted. Additionally, the `safeTransferFrom()` function implemented by the Openzeppeline library will trigger the `onERC721Received` function of the receiver (if the receiver is a contract). In this case, the attacker could reenter the contract before completing the order.

The linked functions which are never called internally within the contract should have external visibility for gas optimization.

According to the code implementation, the `register()` function will update `codes[]`, `_c2a[]` and `codesInUse`. However, the corresponding function `registerUser()` performs the same functionality, yet only can be invoked by the `_owner`. Additionally, if `registerUser()` can be called by anyone, an attacker can set his address as "referral" and claim the "bonus" from `EbisusBayMembership` contract. Therefore, we assume that the `register()` function should also enforce access control.

There should always be events emitted in the sensitive functions that are controlled by centralization roles. In the linked locations, events should be emitted for sensitive functions. For example, - `updatePrice()` - `withdraw()`

The current `registerUser()` function allows the same `_referrer` address to be replaced by difference `_code` at a time. Namely, the owner can update the `_code` of a given referrer by invoking the `registerUser()` function with `_code` that is not in use. ```solidity function registerUser(address _referrer, bytes32 _code) public onlyOwner { require(!codesInUse.contains(_code), "Code in use"); codesInUse.add(_code); codes[_referrer] = _code; _c2a[_code] = _referrer; emit Registered(_referrer, _code); } ``` Since the `require` statement only checks whether the `_code` is in use, the function will add a new code in use and update `codes[_referrer] = _code`.

The contract has unlocked compiler version. An unlocked compiler version in the source code of the contract permits the user to compile it at or above a particular version. This, in turn, leads to differences in the generated bytecode between compilations due to differing compiler version numbers. This can lead to an ambiguity when debugging as compiler specific bugs may occur in the codebase that would be hard to identify over a span of multiple compiler versions rather than a specific one.