Skip to content
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

Remove deposit "on behalf of" [audit] #230

Merged
merged 1 commit into from
Jul 5, 2023

Conversation

grod220
Copy link
Contributor

@grod220 grod220 commented Jul 4, 2023

Currently, when a new asset is listed, someone can deposit on behalf of Credit Manager. If they do this before any other lends have occurred there, they can cause it to throw errors for subsequent lenders via Credit Manager.

The reason is, Credit Manager is not expecting someone to be able to deposit to Red Bank on behalf of itself. After discussing with Piotr and others, we cannot think of a good reason why this functionality exists.

This PR removes on_behalf_of field from Red Bank, solving the Credit Manager vulnerability at the same time.

Note:: Running fmt/clippy found a few things, hence some unrelated code fixes/formatting.

@grod220 grod220 force-pushed the MP-2840-lend-action-can-permanently-fail branch from 85dd7e5 to d685525 Compare July 4, 2023 06:37
@grod220 grod220 requested review from piobab and brimigs July 4, 2023 06:56
@grod220 grod220 changed the title Remove deposit "on behalf of" Remove deposit "on behalf of" [audit] Jul 4, 2023
@grod220 grod220 requested a review from thec00n July 4, 2023 06:57
Copy link
Contributor

@piobab piobab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

on_behalf_of for deposit was for normal users too but I think it is not used in this case too.

@grod220 grod220 merged commit c322e18 into develop Jul 5, 2023
4 checks passed
@grod220 grod220 deleted the MP-2840-lend-action-can-permanently-fail branch July 5, 2023 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants