# Withdraw Expired Request Vulnerability

### Background
The withdraw function behaves differently whether the claimer or the challengers won. Since there is only one claimer, if the claimer is the winner, withdraw can be called and the contract can resolve the claim and send all the stakes to the claimer. In fact, anybody can call withdraw and the claim will be resolved correctly.
For the case of the challengers, however, there can be multiple and the stakes for each winning challenger needs to be computed per challenger.
This results in the fact, that the withdraw is split per challenger (basically each challenger will call withdraw to withdraw its stakes).
(Side note: if we wanted to allow withdrawing for all challengers in one transaction, we would need to limit the total number of possible challengers, which again would open up a more severe attack vector, the very one why we allow multiple challengers).

#### Active Claims
In the contracts there exists one specific property in the requests struct, called `activeClaims`. It counts the number of claims opened. Once a claim is finalized (either L1 or expiration), the claim effectively becomes inactive (semantically). However, a contract needs to be triggered to do something. We decrement `activeClaims` whenever the withdraw function is called for the first time on a claim.

### Attack

Back to the topic: Now, let's assume a request did not get filled. That's the only time when `activeClaims` becomes relevant. All challengers have to withdraw on their own. Attacker(s) could just leave the stakes in the contract, and not call withdraw at all. This would result in the underlying request having an active claim.
The scenario we are talking about only concerns requests which did not get filled, because this is the only scenario where the property `acitveClaims` becomes relevant. It is also what is called a `griefing attack`. The attacker cannot benefit from it (directly), but with little cost it can do great harm to others.
Cost of the attack is only the opportunity cost of the claimStakes, and the harm is the user's tokens locked. Also of course, the reputation of the bridge is also a damage which can result out of this.

#### Example
Rich Richard wants to bridge 1000000 USDC. None of the agents want or can fill this request. So he is entitled to withdraw its tokens after the request's expiry. Now, Evil Alice claims that she filled the request. With another address she actually challenges herself. So from a protocol point of view, everything looks fine. An invalid claim gets challenged, challenger is winning, tokens are secured, everybody happy.
The challenge expires, challenger wins, all good. But now, Evil Alice in the role of the challener decides not to withdraw the claimStakes. Thus `activeClaims` remains `1`. Rich Richard is not able to withdraw its 1M USDC because there is still an active claim marked in the contract even though the contract is semantically finalized and the outcome of the claim is clear.

### Problem
I think the main problem lies in the implementation. The protocol is fine, because the claim eventually finalizes. However, we only "update" the activeClaims property on a specific occasion, namely when the challenger calls withdraw the first time. Additionally, we rely on the challenger to call it. So this results in a single point of failure (similar to what we have seen with claiming). If the challenger doesn't want to or cannot, the funds are locked forever.

### Solution Options
I think what we need to achieve is, making it possible to update the `activeClaims` property whenever a claim becomes finalized. Additionally, this should be permissionless. Anybody should be able to update the value.

As I see it, we could implement it in two different ways:

1) Allow withdraw on behalf of challengers (https://github.com/beamer-bridge/beamer/issues/613). This is a similar solution to claiming on behalf of others. So if the challengers win, we could allow anybody to withdraw for a specific challenger. With that the contract implicitly decrements the activeClaims parameter.
2) Explicitly allowing to update `activeClaims`. Expose a function to the public which allows to check, based on a claimId, whether this claim is finalized. If it is finalized, we decrement `activeClaims`. This involves additional logic that makes sure that one particular claim can only decrement the counter once.

IMO 1) seems to be the easier implementation, 2) the cleaner implementation. Currently, upon withdraw, we implicitly decrement the counter and the check whether it was already decremented is done whether some stake of the claim was already withdrawn. If we strictly separate updating `activeClaims` and withdraw, we need to find a new way, some additional logic or state, which makes sure that `activeClaims` is only ever decremented once per claim.
The withdraw function does that for us already and allowing to withdraw on behalf of others would solve the problem naturally.

In order to give my recommendation, I have to think about the complexity of implementing 2) a bit more. Usually, I tend to lean towards the cleaner solutions but under consideration of the current timeline we have, I might also be fine with 1), which solves the issue with an easy extension. I would probably go with 1) right now, and consider 2) for the future.