Summary
TractorFacet will always return the wrong value for getCounter and updateCounter functions. But those can only work while running through a Blueprint call(while the activePublisher is set). This will prevent users to make external calls to the TractorFacet and consult their data related to the Tractor counter.
This issue occurs because the protocol is trying to fetch the activePublisher from tractorStorage() which has the address set to 1 and this address only changes when the activePublisher is set inside the runBlueprint modifier.
Note that those functions are external and should return the correct value when the msg.sender matches the address of the publisher for that counterId.
function getCounter(bytes32 counterId) public view returns (uint256 count) {
return
LibTractor._tractorStorage().blueprintCounters[
@> LibTractor._tractorStorage().activePublisher
][counterId];
}
* @notice Update counter value.
* @return count New value of counter
*/
function updateCounter(
bytes32 counterId,
LibTractor.CounterUpdateType updateType,
uint256 amount
) external returns (uint256 count) {
uint256 newCount;
if (updateType == LibTractor.CounterUpdateType.INCREASE) {
newCount = getCounter(counterId).add(amount);
} else if (updateType == LibTractor.CounterUpdateType.DECREASE) {
newCount = getCounter(counterId).sub(amount);
}
LibTractor._tractorStorage().blueprintCounters[
@> LibTractor._tractorStorage().activePublisher
][counterId] = newCount;
return newCount;
}
Impact
Users will not be able to update their Blueprint's counter as the updateCounter will set the counterId and its value to the address(1)
Users will not be able to fetch their Blueprint's counter value as the getCounter will try to fetch the value from the address(1)
PoC
Add the following functions to TractorFacet to facilitate the testing and then add TractorFacet to BeanstalkDeployer.
function setPublisher(address payable publisher) external {
LibTractor._setPublisher(publisher);
}
function resetPublisher() external {
LibTractor._resetPublisher();
}
// BeanstalkDeployer
string[] facets = [
...
+ "TractorFacet"
]
Now replace all the code on Tractor.t.sol with the following:
pragma solidity ^0.8.20;
import "forge-std/Test.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {C} from "contracts/C.sol";
import {LibAppStorage} from "contracts/libraries/LibAppStorage.sol";
import {LibTractor} from "contracts/libraries/LibTractor.sol";
import {IBeanstalk} from "contracts/interfaces/IBeanstalk.sol";
import {TokenFacet} from "contracts/beanstalk/farm/TokenFacet.sol";
import {TractorFacet} from "contracts/beanstalk/farm/TractorFacet.sol";
import {TestHelper} from "test/foundry/utils/TestHelper.sol";
import {LibTransfer} from "contracts/libraries/Token/LibTransfer.sol";
import {LibClipboard} from "contracts/libraries/LibClipboard.sol";
import {AdvancedFarmCall} from "contracts/libraries/LibFarm.sol";
import {LibBytes} from "contracts/libraries/LibBytes.sol";
contract TractorTest is TestHelper {
IBeanstalk beanstalk;
IERC20 bean;
TractorFacet tractorFacet;
TokenFacet tokenFacet;
function setUp() public {
initializeBeanstalkTestState(true, false);
beanstalk = IBeanstalk(BEANSTALK);
bean = IERC20(C.BEAN);
tokenFacet = TokenFacet(BEANSTALK);
tractorFacet = TractorFacet(BEANSTALK);
}
function testTractor_whenCallingGetCounter_willReturnWrongCounter() public {
bytes32 counterId = keccak256(abi.encodePacked("counterId"));
_setPublisherWith(counterId);
uint256 newValue = tractorFacet.getCounter(counterId);
assertEq(newValue, 500);
}
function _setPublisherWith(bytes32 counterId) internal {
tractorFacet.setPublisher(payable(address(this)));
tractorFacet.updateCounter(counterId, LibTractor.CounterUpdateType.INCREASE, 500);
uint256 value = tractorFacet.getCounter(counterId);
assertEq(value, 500);
tractorFacet.resetPublisher();
}
}
Run: forge test --match-test testTractor_whenCallingGetCounter_willReturnWrongCounter -vv
Output:
Ran 1 test for test/foundry/tractor/Tractor.t.sol:TractorTest
[FAIL. Reason: assertion failed: 0 != 500] testTractor_whenCallingGetCounter_willReturnWrongCounter() (gas: 56703)
Tools Used
Manual Review and Foundry
Recommendations
Replace LibTractor._tractorStorage().activePublisher with LibTractor._user().
This works for both cases: active publisher set and msg.sender.
✅ Test with the fix:
[PASS] testTractor_whenCallingGetCounter_willReturnWrongCounter() (gas: 52451)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 248.29ms (444.13µs CPU time)
