code-423n4 / 2023-04-eigenlayer-findings

1 stars 1 forks source link

`expiry` should be > `block.timestamp()` rather then >= #430

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/Layr-Labs/eigenlayer-contracts/blob/dbeb85bcd0476e06b8feebf07e33f8a53d54c029/src/contracts/core/StrategyManager.sol#L263

Vulnerability details

Impact

expiry should be > block.timestamp() rather then >= the signature should expire at the timestamp, not when the timestamp is over, when the timestamp reaches, the signature should expire

Proof of Concept

In the function depositIntoStrategyWithSignature there is a condition that is checking if expiry >= block.timestamp but it should be checking if expiry > block.timestamp instead. the signature should expire when the block.timestamp reaches

Tools Used

manual review

Recommended Mitigation Steps

change it to expiry > block.timestamp

Assessed type

Invalid Validation

0xSorryNotSorry commented 1 year ago

Is it a bug or a feature point like the zero index is the first element. In addition, the submission does not provide any demonstration of the issue, reasoning and code blocks, security concern behind it.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as primary issue

c4-sponsor commented 1 year ago

Sidu28 marked the issue as sponsor disputed

Sidu28 commented 1 year ago

The definition is semantically ambiguous about whether this check should be strict or not. A time difference of 1 second in signature expiry will not have a serious impact regardless.

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)

GalloDaSballo commented 1 year ago

Downgrading to QA, but may close this

Nabeel-javaid commented 1 year ago

actually this should be marked as medium cause the delay of 1 seconds can cause many issues, as we do have a history to justify that. so ig this should be medium