Replies: 3 comments 7 replies
-
|
You can refer to https://docs.cyfrin.io/aderyn-custom-detectors/detectors-quickstart for customizing the detectors : ) |
Beta Was this translation helpful? Give feedback.
-
|
A quick way to make us aware of false-positives is to open an issue on this repo @chista0x :) we're always looking for these! |
Beta Was this translation helpful? Give feedback.
-
|
Of course, creating an issue is the solution, but I wanted to help in this issue. In the contribution section, there is a tutorial on how to create a new detector, but how to improve the previous detectors is a question for me. Should I get a fork and send a pull request after changing? Or should I create an issue first and then request to assign it to myself, and then fork and pull request ? Aren't these two photos I posted false-positive? Do I need to create a new issue for these? |
Beta Was this translation helpful? Give feedback.
-
|
Can you confirm if the first (unsafe erc 20 function) issue still persists ? (In the latest Aderyn version) |
Beta Was this translation helpful? Give feedback.
-
At the moment, there is no formal approach in place (Could be, in the future). Please feel free to create an issue, work on it and send a PR. You are always welcome to do so. Happy to take a look : ) |
Beta Was this translation helpful? Give feedback.
-
|
Unfortunately, there are still errors in the new version
|
Beta Was this translation helpful? Give feedback.
-
|
I compared the Ast build data which is as follows: contract C1 {
function withdraw() external {
payable(msg.sender).transfer(1);
}
} "expression": {
"argumentTypes": [
{
"typeIdentifier": "t_rational_1_by_1",
"typeString": "int_const 1"
}
],
"expression": {
"arguments": [
{
"expression": {
"id": 46745,
"name": "msg",
"nodeType": "Identifier",
"overloadedDeclarations": [],
"referencedDeclaration": -15,
"src": "123:3:26",
"typeDescriptions": {
"typeIdentifier": "t_magic_message",
"typeString": "msg"
}
},
"id": 46746,
"isConstant": false,
"isLValue": false,
"isPure": false,
"lValueRequested": false,
"memberLocation": "127:6:26",
"memberName": "sender",
"nodeType": "MemberAccess",
"src": "123:10:26",
"typeDescriptions": {
"typeIdentifier": "t_address",
"typeString": "address"
}
}
],
"expression": {
"argumentTypes": [
{
"typeIdentifier": "t_address",
"typeString": "address"
}
],
"id": 46744,
"isConstant": false,
"isLValue": false,
"isPure": true,
"lValueRequested": false,
"nodeType": "ElementaryTypeNameExpression",
"src": "115:8:26",
"typeDescriptions": {
"typeIdentifier": "t_type$_t_address_payable_$",
"typeString": "type(address payable)"
},
"typeName": {
"id": 46743,
"name": "address",
"nodeType": "ElementaryTypeName",
"src": "115:8:26",
"stateMutability": "payable",
"typeDescriptions": {}
}
},
"id": 46747,
"isConstant": false,
"isLValue": false,
"isPure": false,
"kind": "typeConversion",
"lValueRequested": false,
"nameLocations": [],
"names": [],
"nodeType": "FunctionCall",
"src": "115:19:26",
"tryCall": false,
"typeDescriptions": {
"typeIdentifier": "t_address_payable",
"typeString": "address payable"
}
},
"id": 46748,
"isConstant": false,
"isLValue": false,
"isPure": false,
"lValueRequested": false,
"memberLocation": "135:8:26",
"memberName": "transfer",
"nodeType": "MemberAccess",
"src": "115:28:26",
"typeDescriptions": {
"typeIdentifier": "t_function_transfer_nonpayable$_t_uint256_$returns$__$",
"typeString": "function (uint256)"
}
},
contract C2 {
function withdraw() external {
IERC20(msg.sender).transfer(address(0), 1);
}
}
interface IERC20 {
function transfer(address to, uint256 value) external returns (bool);
}
"expression": {
"argumentTypes": [
{
"typeIdentifier": "t_address",
"typeString": "address"
},
{
"typeIdentifier": "t_rational_1_by_1",
"typeString": "int_const 1"
}
],
"expression": {
"arguments": [
{
"expression": {
"id": 5,
"name": "msg",
"nodeType": "Identifier",
"overloadedDeclarations": [],
"referencedDeclaration": -15,
"src": "122:3:0",
"typeDescriptions": {
"typeIdentifier": "t_magic_message",
"typeString": "msg"
}
},
"id": 6,
"isConstant": false,
"isLValue": false,
"isPure": false,
"lValueRequested": false,
"memberLocation": "126:6:0",
"memberName": "sender",
"nodeType": "MemberAccess",
"src": "122:10:0",
"typeDescriptions": {
"typeIdentifier": "t_address",
"typeString": "address"
}
}
],
"expression": {
"argumentTypes": [
{
"typeIdentifier": "t_address",
"typeString": "address"
}
],
"id": 4,
"name": "IERC20",
"nodeType": "Identifier",
"overloadedDeclarations": [],
"referencedDeclaration": 28,
"src": "115:6:0",
"typeDescriptions": {
"typeIdentifier": "t_type$_t_contract$_IERC20_$28_$",
"typeString": "type(contract IERC20)"
}
},
"id": 7,
"isConstant": false,
"isLValue": false,
"isPure": false,
"kind": "typeConversion",
"lValueRequested": false,
"nameLocations": [],
"names": [],
"nodeType": "FunctionCall",
"src": "115:18:0",
"tryCall": false,
"typeDescriptions": {
"typeIdentifier": "t_contract$_IERC20_$28",
"typeString": "contract IERC20"
}
},
"id": 8,
"isConstant": false,
"isLValue": false,
"isPure": false,
"lValueRequested": false,
"memberLocation": "134:8:0",
"memberName": "transfer",
"nodeType": "MemberAccess",
"referencedDeclaration": 27,
"src": "115:27:0",
"typeDescriptions": {
"typeIdentifier": "t_function_external_nonpayable$_t_address_$_t_uint256_$returns$_t_bool_$",
"typeString": "function (address,uint256) external returns (bool)"
}
},I think that you should use the "typeDescriptions": {
"typeIdentifier": "t_function_transfer_nonpayable$_t_uint256_$returns$__$",
"typeString": "function (uint256)"
} "typeDescriptions": {
"typeIdentifier": "t_function_external_nonpayable$_t_address_$_t_uint256_$returns$_t_bool_$",
"typeString": "function (address,uint256) external returns (bool)"
}Or use the child expression typeIdentifier "typeDescriptions": {
"typeIdentifier": "t_address_payable",
"typeString": "address payable"
} "typeDescriptions": {
"typeIdentifier": "t_function_transfer_nonpayable$_t_uint256_$returns$__$",
"typeString": "function (uint256)"
} |
Beta Was this translation helpful? Give feedback.
-
|
Thank you so much @chista0x . I now see why it's an error, it's because we are only checking for member accesses with the name "transfer". (Which means, we aren't checking if that |
Beta Was this translation helpful? Give feedback.
-
|
Track here #556 |
Beta Was this translation helpful? Give feedback.
-
|
The zero address part requires tuning and that can be tracked here #556 |
Beta Was this translation helpful? Give feedback.
-
I have a simple code that I've tested on the analyzer, but I didn't get accurate output reports. I want to know how I can improve existing detectors. Is there any tutorial available on this topic?
Beta Was this translation helpful? Give feedback.
All reactions