Weak Conditional Statements which gives Room for Unauthorize Execution of unfollow() and _followWithWrappedToken() by an attacker

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L118-L125 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L319-L327

Vulnerability details

Impact

From L118-L125 & L319-L327 of FollowNFT.sol contract, unfollow() and _followWithWrappedToken() functions are performing 4 and 5 "logical AND" operations(&&) respectively. Using "logical AND" operations in coding is generally seen as strength and intensity but in these cases it is weakness! as it is having more negative effect than positive due the tough constrains needed to revert when a user/attacker "Does not have Permission", this is because every one of those conditions must be met before a reversion occurs. To put it in simple terms an attacker simply needs to pass 1/4 & 1/5 of the "logical AND" operations to complete execution of these functions without reversion.

Proof of Concept

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L118-L125 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L319-L327

119. if (
120.  (followTokenOwner != unfollowerProfileOwner) &&
121.  (followTokenOwner != transactionExecutor) &&
122.  !isApprovedForAll(followTokenOwner, transactionExecutor) &&
123.   !isApprovedForAll(followTokenOwner, unfollowerProfileOwner)
124.   ) {
125.       revert DoesNotHavePermissions();
126.    }
...
319.  if (
320.  !isFollowApproved &&
321.   followTokenOwner != followerProfileOwner &&
322.  followTokenOwner != transactionExecutor &&
323.  !isApprovedForAll(followTokenOwner, transactionExecutor) &&
324.  !isApprovedForAll(followTokenOwner, followerProfileOwner)
325.  ) {
326.      revert DoesNotHavePermissions();
327.  }
``
using unfollow(...) of L103-L128 of FollowNFT.sol contract as an example
1. Alice the legit owner of a profileNft abc has no intention of unfollowing.
2. Bob(attacker) calls the unfollow(uint256 unfollowerProfileId, address transactionExecutor) function using Alice's followerProfileId and Alice's Address as parameters.
3. All neccessary checks are passed until L119 where reversion with DoesNotHavePermissions() is possible if all the "logical AND" operations turn out true.
4. However, L121.  (followTokenOwner != transactionExecutor) would turn out false, which gives Bob the opportunity to complete the execution of the unfollow(...) function without permission

## Tools Used
solidity, hardhat
## Recommended Mitigation Steps
using msg.sender instead of transactionExecutor at L121 would be one way but I recommend working with "||" and "&&" together if possible instead of just "&&" operator

## Assessed type

Access Control

[c4-pre-sort]

141345 marked the issue as low quality report

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid

[Picodes]

" (followTokenOwner != transactionExecutor) would turn out false" -> then followTokenOwner == transactionExecutor so what is the issue? This is very unclear.