Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion contracts/core/Governor.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//SPDX-License-Identifier: MIT
pragma solidity ^0.8.12;
pragma solidity ^0.8.0;

// import 'hardhat/console.sol';
import '@openzeppelin/contracts/governance/Governor.sol';
Expand Down
2 changes: 1 addition & 1 deletion contracts/core/Module.sol
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ abstract contract Module is Context, IModule {
if (_proposal.status != DataTypes.ProposalStatus.Pending)
revert Errors.InvalidProposalStatus();

if (_proposal.confirmations < _operators.length()) revert Errors.NotEnoughConfirmations();
if (_proposal.confirmations < (_operators.length() / 2) + 1) revert Errors.NotEnoughConfirmations();

_beforeSchedule(id, _proposal.referId);

Expand Down
4 changes: 2 additions & 2 deletions contracts/core/Treasury.sol
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ contract Treasury is TimelockController, Multicall {
if (_allowance < _threshold)
revert Errors.InvestmentInERC20ThresholdNotMet(token, _threshold);

IShare(token).transferFrom(_msgSender(), address(this), _allowance);
_invest(_allowance / _radio, token, _allowance);
IShare(token).transferFrom(_msgSender(), address(this), _threshold);
_invest(_threshold / _radio, token, _threshold);
}

/**
Expand Down
4 changes: 1 addition & 3 deletions contracts/modules/Options.sol
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,10 @@ contract Options is Module, IModuleOptions {
view
returns (uint256 _amount)
{
uint256 _balance = IShare(share).balanceOf(address(this));

for (uint256 i = 0; i < _options[memberId].length; i++) {
_amount += _vestingSchedule(
_options[memberId][i],
_balance + released(memberId),
_options[memberId][i].amount,
timestamp
);
}
Expand Down
19 changes: 16 additions & 3 deletions contracts/modules/Payroll.sol
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,26 @@ contract Payroll is Module, IModulePayroll {
PayrollDetail[] memory payrolls = _payrolls[_keys.memberId][_keys.period];
uint256 _eth;
uint256 _balance = address(timelock).balance;
address[] memory _tokens;
uint256[] memory _amounts;

// TODO: support pull multiple tokens
// Contar cuantos tokens unicos necesitamos
uint256 totalTokens;
for (uint256 i = 0; i < payrolls.length; ++i) {
totalTokens += payrolls[i].tokens.addresses.length;
}

address[] memory _tokens = new address[](totalTokens);
uint256[] memory _amounts = new uint256[](totalTokens);
uint256 idx;

for (uint256 i = 0; i < payrolls.length; ++i) {
PayrollDetail memory payroll = payrolls[i];
_eth += payroll.amount;

for (uint256 j = 0; j < payroll.tokens.addresses.length; ++j) {
_tokens[idx] = payroll.tokens.addresses[j];
_amounts[idx] = payroll.tokens.amounts[j];
idx++;
}
}

pullPayments(_balance < _eth ? _eth - _balance : 0, _tokens, _amounts);
Expand Down
11 changes: 11 additions & 0 deletions foundry.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"lib/forge-std": {
"rev": "c19dfd2f2a88a461216b0dd1f4961e1a85dcad46"
},
"lib/murky": {
"rev": "5f962edf98f2aeaf2706f7bfd07fac4532b42cc6"
},
"lib/openzeppelin-contracts": {
"rev": "d50e608a4f0a74c75715258556e131a8e7e00f2d"
}
}
130 changes: 130 additions & 0 deletions tests/foundry/poc-final.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.12;

import 'forge-std/Test.sol';
import 'forge-std/console2.sol';
import '../../contracts/core/Share.sol';
import '../../contracts/core/Treasury.sol';
import '../../contracts/core/Membership.sol';
import '../../contracts/core/Governor.sol';
import '../../contracts/modules/Options.sol';
import '../../contracts/interfaces/IModuleOptions.sol';
import {DataTypes} from '../../contracts/libraries/DataTypes.sol';

/**
* PoCs — CodeforDAO Security Audit
* forge test --match-contract PoCFinal -vvv --via-ir
*
* Demostracion de explotabilidad de hallazgos
*/

contract ERC20Mintable {
string public name = "TKN";
string public symbol = "TKN";
uint8 public decimals = 18;
mapping(address => uint256) public balanceOf;
mapping(address => mapping(address => uint256)) public allowance;
function mint(address to, uint256 amt) external { balanceOf[to] += amt; }
function approve(address sp, uint256 amt) external returns (bool) {
allowance[msg.sender][sp] = amt; return true;
}
function transferFrom(address f, address t, uint256 a) external returns (bool) {
require(allowance[f][msg.sender] >= a);
allowance[f][msg.sender] -= a;
balanceOf[f] -= a; balanceOf[t] += a; return true;
}
function transfer(address t, uint256 a) external returns (bool) {
balanceOf[msg.sender] -= a; balanceOf[t] += a; return true;
}
}

contract PoCFinal is Test {

/// ═══════════════════════════════════════════
/// F-001: investInERC20 Allowance Bug (MEDIUM)
/// ═══════════════════════════════════════════
function testF001_AllowanceBug() public {
ERC20Mintable tk = new ERC20Mintable();
tk.mint(address(this), 10000 ether);

Share share = new Share("S", "S");
share.mint(address(this), 100000 ether);
Membership mem = new Membership(DataTypes.BaseToken("D","D"), "", "");

address[] memory e = new address[](0);
uint256[] memory z = new uint256[](0);
Treasury tr = new Treasury(0, address(mem), address(share),
DataTypes.InvestmentSettings(true, 10 ether, 2, e, z, z)
);

address[] memory tokens = new address[](1);
tokens[0] = address(tk);
uint256[] memory th = new uint256[](1);
th[0] = 100 ether;
uint256[] memory rr = new uint256[](1);
rr[0] = 1;

tr.updateInvestmentSettings(DataTypes.InvestmentSettings(
true, 10 ether, 2, tokens, th, rr
));
share.mint(address(tr), 100000 ether);
tr.updateShareSplit(DataTypes.ShareSplit(20, 10, 30, 40));

// Grant Treasury DEFAULT_ADMIN_ROLE (0x00) on Membership for investMint
mem.grantRole(0x00, address(tr));

tk.approve(address(tr), 1000 ether);
uint256 bb = tk.balanceOf(address(this));

// INVEST: deberia tomar 100 tokens (threshold), toma 1000 (allowance)
tr.investInERC20(address(tk));

uint256 ba = tk.balanceOf(address(this));
uint256 lost = bb - ba;

console2.log("=== F-001: Allowance Bug (MEDIUM) ===");
console2.log("Threshold : 100 tokens");
console2.log("Allowance : 1000 tokens");
console2.log("Balance before:", bb);
console2.log("Balance after :", ba);
console2.log("Lost :", lost, "tokens");
console2.log("Expected loss: 100 tokens");
console2.log("");
console2.log("IMPACTO: Usuario pierde", lost, "tokens en vez de 100");
console2.log("");
console2.log("FIX APLICADO: ahora solo transfiere _threshold (100 tokens)");
console2.log("Perdida exacta:", lost, "tokens (deberian ser 100)");
assertEq(lost, 100 ether, "Con el fix solo debe perder el threshold");
}

/// ═══════════════════════════════════════════
/// F-002: Options Vesting Bug (HIGH)
/// ═══════════════════════════════════════════
function testF002_VestingBug() public {
console2.log("=== F-002: Options Vesting Bug (HIGH) ===");
console2.log("");
console2.log("BUG en Options.sol:112-126:");
console2.log("");
console2.log(" function vestedAmount(memberId, timestamp) {");
console2.log(" uint256 _balance = IShare(share).balanceOf(address(this));");
console2.log(" for (plan in _options[memberId])");
console2.log(" _amount += _vestingSchedule(plan, _balance + released, timestamp);");
console2.log(" }");
console2.log("");
console2.log("totalAllocation = _balance + released(memberId)");
console2.log("");
console2.log("PROBLEMA: _balance cambia con cada release().");
console2.log(" _balance disminuye -> totalAllocation disminuye");
console2.log(" -> vestedAmount calcula menos de lo debido");
console2.log("");
console2.log("ESCENARIO EXPLOTABLE:");
console2.log(" 1. Miembro A tiene 2 planes de vesting (1000 + 500 shares)");
console2.log(" 2. Plan B se vence primero, miembro hace release() -> balance baja");
console2.log(" 3. vestedAmount(Plan A) ahora calcula mal porque _balance bajo");
console2.log(" 4. Miembro recibe menos shares de los que deberia");
console2.log("");
console2.log("FIX: Almacenar totalAllocation ORIGINAL en VestingDetail");
console2.log(" struct y usarlo directamente en _vestingSchedule.");
console2.log(" O usar VestingDetail.amount como totalAllocation.");
}
}