QA Report #91
Labels
bug
Something isn't working
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
resolved
Finding has been patched by sponsor (sponsor pls link to PR containing fix)
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Summary
We list 2 non-critical findings here:
In conclusion, it's better to check the validity of the mapping’s key. And using a safe version of ERC721 functions is usually a better choice.
(Non) loanEndSeconds does not check valid loanId
Impact
In
NFTLoanFacilitator.sol/loanEndSeconds()
, it doesn’t check whether loanId is validProof of Concept
https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L363
Recommended Mitigation Steps
Add check for loanId
(Non) Should use safe version of ERC721 functions
Impact
In
NFTLoanFacilitator.sol
, it usesIERC721Mintable(borrowTicketContract).mint
which is not a good practice.Proof of Concept
https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L102
https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L161
Recommended Mitigation Steps
Use
safeMint
instead ofmint
The text was updated successfully, but these errors were encountered: