TrungOre - Fee in contract FixedStrikeOptionTeller can be bypassed by the protocol integrate with Bond

[github-actions]

TrungOre

high

Fee in contract FixedStrikeOptionTeller can be bypassed by the protocol integrate with Bond

Summary

See the detail

Vulnerability Detail

The FixedStrikeOptionTeller.exercise() function is employed to exercise an optionToken by supplying the necessary quote tokens. In return, the sender receives a specific quantity of payout tokens. Within this function, the sender incurs a fee, which amounts to protocolFee / FEE_DECIMALS of the quote tokens they receive.

/// link = https://github.com/sherlock-audit/2023-06-bond/blob/main/options/src/fixed-strike/FixedStrikeOptionTeller.sol#L347-L348
uint256 fee = (quoteAmount * protocolFee) / FEE_DECIMALS/*1e5*/;
fees[quoteToken] += fee;

However, there is an exception when the sender is the same as the receiver; in this case, the fee will be skipped.

• https://github.com/sherlock-audit/2023-06-bond/blob/main/options/src/fixed-strike/FixedStrikeOptionTeller.sol#L341-L342

It is important to note that the ownership of the FixedStrikeOptionTeller contract lies with Bond. However, the OTLM contracts can be generated by a third party for the distribution of their tokens. This third party has the ability to deploy a contract and designate it as the receiver for the OTLM contracts. By creating this receiver contract, the third party establishes a potential loophole for their users to circumvent Bond's protocol fee when exercising the optionToken. This can be achieved by incorporating a function within the receiver contract to interact with the FixedStrikeOptionTeller.exercise() function. The process of this function can be described as follows:

• transfer optionToken from sender to receiver contract.
• transfer quoteToken from sender to receiver contract.
• call FixedStrikeOptionTeller.exercise().
• transfer payoutToken from receiver contract to the sender.

Impact

There won't be any fee accured for the bond protocol

Code Snippet

https://github.com/sherlock-audit/2023-06-bond/blob/main/options/src/fixed-strike/FixedStrikeOptionTeller.sol#L341-L361

Tool used

Manual Review

Recommendation

Consider to charge fee when calling function FixedStrikeOptionTeller.exercise() even the sender is the receiver.

[trungore]

Escalate
I believe this issue to be valid, leading to the loss of fees for the bond protocol.

Evidence supporting the claim that the FixedStrikeOptionTeller is controlled by the Bond team can be found in the Sherlock Discord community:

• Inquiry: Link to Ask
• Response: Link to Answer

Furthermore, the OTLM contract can be deployed by anyone using the OTLMFactory. While users are responsible for interacting with the "trusted" OTLM, the issue describes how the OTLM doesn't harm users but instead creates a backdoor to help them bypass fees from the Bond team, resulting in a loss of fees for Bond.

I believe this issue is valid and cause the loss of fee for bond protocol.

[sherlock-admin]

Escalate
I believe this issue to be valid, leading to the loss of fees for the bond protocol.

Evidence supporting the claim that the FixedStrikeOptionTeller is controlled by the Bond team can be found in the Sherlock Discord community:

• Inquiry: Link to Ask
• Response: Link to Answer

Furthermore, the OTLM contract can be deployed by anyone using the OTLMFactory. While users are responsible for interacting with the "trusted" OTLM, the issue describes how the OTLM doesn't harm users but instead creates a backdoor to help them bypass fees from the Bond team, resulting in a loss of fees for Bond.

I believe this issue is valid and cause the loss of fee for bond protocol.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[ctf-sec]

Bringing for sponsor review, the recommendation maybe valuable, will sync the protocol's thoughts

[ctf-sec]

Comment from sponsor:

This is by design. We have the exclusion for the receiver so that they can “reclaim” tokens that they don’t distribute before expiry if needed. We only take a fee on a successful exercise

[trungore]

Escalate
I acknowledge that the receiver's ability to "reclaim" tokens is intentional, but the issue persisted despite the sponsor's comment.

If every protocol were to implement a similar backdoor as described in the issue, it would result in no fees being accrued for the bond team, leading to a loss of funds.

• This attack can be executed at a very low cost by simply deploying a basic "receiver" contract.
• Since protocols integrated with Bond can attract more users with lower fees, there is a likelihood that they would deploy this "receiver" contract.

--> Hence, based on Sherlock's judging criteria, it remains a valid and significant issue.

If the protocol still wishes to exclude fees for the "receiver," one recommendation is to designate the "receiver" as an externally owned account (EOA) wallet and require tx.origin == receiver when the receiver invokes exercise().

[sherlock-admin]

Escalate
I acknowledge that the receiver's ability to "reclaim" tokens is intentional, but the issue persisted despite the sponsor's comment.

If every protocol were to implement a similar backdoor as described in the issue, it would result in no fees being accrued for the bond team, leading to a loss of funds.

• This attack can be executed at a very low cost by simply deploying a basic "receiver" contract.
• Since protocols integrated with Bond can attract more users with lower fees, there is a likelihood that they would deploy this "receiver" contract.

--> Hence, based on Sherlock's judging criteria, it remains a valid and significant issue.

If the protocol still wishes to exclude fees for the "receiver," one recommendation is to designate the "receiver" as an externally owned account (EOA) wallet and require tx.origin == receiver when the receiver invokes exercise().

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[Oot2k]

I think this is a design choice, and therefor a low in regards to sherlock rules.

[trungore]

Hi @Oot2k, I respect your comment regarding my escalation.
However, I still strongly believe that this issue is not acknowledged by the protocol.
If it's a design choice from the Bond team and they are already aware of the issue, why do they still have the fee mechanism in their code when all the OTLM contracts can deploy the receiver contract?
It doesn't make sense to me that the fee mechanism was coded but can be easily bypassed, especially if it was intended behavior from the protocol.

Moreover, as I described in the issue, the bond protocol will lose their fee. So it's the high impact, not just low.
An example about bypassing the fee is high severity is from y2k contest:
sherlock-audit/2023-03-Y2K-judging#75

[ctf-sec]

In Y2k contest issue 75, the flaw is in the code itself, there is a code place where the smart contract does not charge fee

While in the finding, it has a strong assumption in the future integration, which is not in scope

https://docs.sherlock.xyz/audits/judging/judging

Future issues: Issues that result out of a future integration/implementation that was not intended (mentioned in the docs/README) or because of a future change in the code (as a fix to another issue) are not valid issues.

transfer optionToken from sender to receiver contract.
transfer quoteToken from sender to receiver contract.
call FixedStrikeOptionTeller.exercise().
transfer payoutToken from receiver contract to the sender.

The external receiver contract maybe take the option token and call exercise directly to pull the payout token to himself

or he can choose to not exercise token wait for the option token to expire and he call reclaim.

so he does not pay the protocol and does not pay the sender as well

Just want to put the sponsor's feedback screenshot here:

[trungore]

While in the finding, it has a strong assumption in the future integration, which is not in scope

Why do u think it's a "future integration" ? It's obvious that the Bond protocol was designed for another third party to integrate with which is not a "integration" at all. I mean the third party here is "user", not any specific integration, i think u have a misunderstanding here.

Future issues: Issues that result out of a future integration/implementation that was not intended (mentioned in the docs/README) or because of a future change in the code (as a fix to another issue) are not valid issues.

I believe that the future integration from the sherlock here is about the change of code when the bond protocol have a implementation that interact directly with a specified protocol (like Aave, Compound, ...) and if the integrated protocol change / update the codebase, it won't count.

or he can choose to not exercise token wait for the option token to expire and he call reclaim.

Who is "he" here ? If u mean "he" is the "receiver" contract, so why the receiver contract "wait for the option token to expire and call reclaim" ? If the "receiver" contract contain that functionality, the sender won't use it anw, because they will lose their benefit of optionToken ?

[hrishibhat]

Tagging @Oighty to have a look at this issue.

[Oighty]

The purpose of the receiver being able to "exercise" early is so that they can "reclaim" collateral for option tokens they still hold without waiting for them to expire (as is required in the reclaim function). We don't want to take a fee on option tokens that are not used.

[trungore]

Hi @Oighty,
The issue described that the third party can deploy a receiver contract here.
So users can use the receiver contract to exercise their optionTokens. So the optionToken in this case is owned by the users not the receiver. And the receiver here just serve only 1 purpose that help users to bypass the bond's fee

[Oighty]

@WelToHackerLand thanks for the clarification. We don't view this as any more risky than someone forking the open source contracts. Appreciate the heads up, but we don't plan to remediate.

[hrishibhat]

Result:
Low
Unique
Considering this a low based on the above comments from the protocol.
Pasting my questions to the Sponsor along with the answer for more context:

Q : 1. You do mention "We only take a fee on a successful exercise". What would be a successful exercise here for which bond should receive a fee?
2. I'm assuming that the protocols that integrate with bond are assumed to be trusted and not bypass the fee. Don't you see it as a loss in case a third party misuses this?

A: 1 : A successful exercise is when the issuer creates an option, it is distributed to a holder, they call exercise on the contract to receive the payout token amount (in case of a call option), the receiver gets the quote token amount, and the protocol gets a fee in the quote token.
2. We make no assumptions about people integrating with the system. Yes, you could write a contract that uses the bond teller to create tokens + custody them which functions as a receiver. External users could use this contract with a passthrough exercise function to avoid paying the protocol fee. In a vacuum is this a loss? Perhaps it is.

However, bypassing the fee is no different than someone forking the open source code, redeploying with a fee of zero, and using the contracts as intended. We intend to use an open source license so we don't think the overall risk is any greater than that.

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• WelToHackerLand: rejected
• WelToHackerLand: rejected