Open code423n4 opened 3 years ago
Valid, although it is intended design to launch with 5, which cannot be reduced after, so disagree with severity.
Our decision matrix for severity:
0: No-risk: Code style, clarity, off-chain monitoring (events etc), exclude gas-optimisations 1: Low Risk: UX, state handling, function incorrect as to spec 2: Funds-Not-At-Risk, but can impact the functioning of the protocol, or leak value with a hypothetical attack path with stated assumptions, but external requirements 3: Funds can be stolen/lost directly, or indirectly if a valid attack path shown that does not have handwavey hypotheticals.
Recommended: 2
Handle
@cmichelio
Vulnerability details
Vulnerability Details
The
Router.getAnchorPrice
sorts thearrayPrices
array and always returns the third element_sortedAnchorFeed[2]
. This only returns the median if_sortedAnchorFeed
is of length 5, but it can be anything from0
toanchorLimit
.Impact
If not enough anchors are listed initially, it might become out-of-bounds and break all contract functionality due to revert, or return a wrong median. If
anchorLimit
is set to a different value than 5, it's also wrong.Recommended Mitigation Steps
Check the length of
_sortedAnchorFeed
and return_sortedAnchorFeed[_sortedAnchorFeed.length / 2]
if it's odd, or the average of the two in the middle if it's even.