-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
beforeTokenTransfer called with wrong parameters in LBToken._burn #108
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
M-02
primary issue
Highest quality submission among a set of duplicates
selected for report
This submission will be included/highlighted in the audit report
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Comments
code423n4
added
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
labels
Oct 17, 2022
0x0Louis
added
the
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
label
Oct 31, 2022
The warden has shown how, due to a typo / programming mistake, the hook for burning tokens is called with incorrect parameters. Because of the caller being the pair, this ends up having reduced impact. |
c4-judge
added
the
primary issue
Highest quality submission among a set of duplicates
label
Nov 11, 2022
GalloDaSballo marked the issue as primary issue |
c4-judge
added
the
selected for report
This submission will be included/highlighted in the audit report
label
Nov 11, 2022
GalloDaSballo marked the issue as selected for report |
This was referenced Nov 14, 2022
Simon-Busch
added
the
satisfactory
satisfies C4 submission criteria; eligible for awards
label
Dec 5, 2022
Marked this issue as Satisfactory as requested by @GalloDaSballo |
Simon-Busch
removed
the
satisfactory
satisfies C4 submission criteria; eligible for awards
label
Dec 5, 2022
Revert, wrong action |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
M-02
primary issue
Highest quality submission among a set of duplicates
selected for report
This submission will be included/highlighted in the audit report
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Lines of code
https://github.com/code-423n4/2022-10-traderjoe/blob/37258d595d596c195507234f795fa34e319b0a68/src/LBToken.sol#L237
Vulnerability details
Impact
In
LBToken._burn
, the_beforeTokenTransfer
hook is called withfrom = address(0)
andto = _account
:Through a lucky coincidence, it turns out that this in the current setup does not cause a high severity issue.
_burn
is always called with_account = address(this)
, which means thatLBPair._beforeTokenTransfer
is a NOP. However, this wrong call is very dangerous for future extensions or protocol that built on top of the protocol / fork it.Proof Of Concept
Let's say the protocol is extended with some logic that needs to track mints / burns. The canonical way to do this would be:
Such an extension would break, which could lead to loss of funds or a bricked system.
Recommended Mitigation Steps
Call the hook correctly:
The text was updated successfully, but these errors were encountered: