2021 년 12 월 16 일
개요
XNUMXD덴탈의 1inch 팀에서 우리에게 검토 및 감사를 요청했습니다. 지정가 주문 프로토콜 v2 스마트 계약. 우리는 코드를 살펴보고 이제 결과를 게시합니다.
범위
커밋을 감사했습니다. 4d94eea25e4dac6271bfd703096a5c4a4d899b4a
의 1inch/limit-order-protocol
저장소. 범위에는 다음과 같은 계약이 포함되었습니다.
- OrderMixin.sol
- OrderRFQMixin.sol
- PredicateHelper.sol
- RevertReasonParser.sol
- Permitable.sol
- ChainlinkCalculator.sol
- ArgumentsDecoder.sol
- AmountCalculator.sol
- NonceManager.sol
- LimitOrderProtocol.sol
- ImmutableOwner.sol
- InteractiveNotificationReceiver.sol
- AggregatorInterface.sol
- IDaiLikePermit.sol
외부 종속성 및 프로젝트, 게임 이론 및 인센티브 디자인과 함께 다른 모든 프로젝트 파일 및 디렉터리(테스트 포함)도 이 감사 범위에서 제외되었습니다. 외부 코드 및 계약 종속성은 문서화된 대로 작동하는 것으로 가정되었으며 1inch에서 제공하는 백엔드 서비스는 프로토콜의 최선의 이익을 위해 작동하는 것으로 가정되었습니다.
전반적인 건강
일반적으로 우리는 프로젝트의 코드베이스가 읽기 쉽고 잘 구성되어 있다는 것을 알았습니다. 특히 어셈블리 코드 블록, 프로토콜의 엣지 케이스, 사용될 자산/술어/외부 리소스와 관련된 보다 광범위한 문서화를 통해 이점을 얻을 수 있었지만, 제공되는 백엔드 서비스의 책임/한계, 행위자 간의 상호 작용. 이 프로젝트는 작업을 효율적으로 수행하기 위해 많은 노력을 기울이고 있으며 때로는 코드를 추론하기 어렵게 만드는 위험도 감수합니다. 이에 관련된 문제를 아래에 제기합니다. 감사 전반에 걸쳐 1inch 팀은 가용성이 높고 응답성이 뛰어나며 협력하기가 매우 쉬웠습니다.
시스템 개요
지정가 주문 프로토콜은 주문을 가능하게 합니다 makers
토큰 스왑을 위해 오프체인 주문에 서명합니다. 그런 다음 프로토콜은 이전에 서명된 주문을 주문별로 작성하는 것을 용이하게 합니다. takers
. 주문은 확장성이 뛰어나며 주문 작성 프로세스 전반에 걸쳐 여러 지점에서 외부 계약을 정적으로 호출할 수 있습니다. 이러한 확장성은 프로토콜에 유용성을 부여하지만 주문 자체에 대한 복잡성과 더 큰 공격 표면을 추가합니다.
주문 세부정보가 온체인에 저장되지 않는다는 점을 기억하는 것이 중요합니다. 주문의 채우기 상태 또는 취소 상태는 주문 해시를 통해서만 추적됩니다. 이를 위해서는 주문이 P1P 또는 중앙화된 당사자를 통해 공유되어야 합니다. 이 경우 XNUMXinch 팀은 서명된 주문을 집계하고 해당 주문을 다른 프로토콜의 유동성 소스로 사용하여 중앙화된 당사자 역할을 하려고 합니다. 주문은 자체 API를 통해 게시되므로 사용자가 주문과 상호 작용할 수 있습니다.
이러한 중앙 집중화를 통해 1inch 팀은 어떤 주문이 게시되고 최종적으로 실행되는지에 대한 극단적인 통제권을 갖게 됩니다. 이는 또한 주문을 검열할 수 있는 기능을 제공합니다. 이는 악의적이거나 기만적인 주문의 경우 유용할 수 있지만, API를 통해 이를 표시하지 않음으로써 유리한 주문의 경우 다른 사용자에게 선점할 수 있고 남용될 수도 있습니다.
권한있는 역할
역할이 사용되는 계약이 범위를 벗어났지만 하나의 권한 있는 역할이 식별되었습니다. 안 immutableOwner
대리계약 생성시 대리계약 작성자로 설정되며, 대리계약에 대한 접근을 제한하는데 사용됩니다. external
기능.
외부 종속성과 신뢰 가정
이 프로토콜의 설계에는 오프체인 및 온체인 구성 요소가 필요하며 이 하이브리드 모델은 보고서에서 확인된 일부 공격 벡터를 완화하는 데 사용될 수 있지만 해당 기능의 비용은 1inch 팀 및 인프라에 대한 의존도를 증가시킵니다.
또한, 지정가 주문 프로토콜은 체인링크 오라클에서 가격을 검색하는 기능을 제공합니다. 우리는 이러한 오라클이 정직하고 접근 가능하며 적절하게 작동한다고 가정했습니다.
또한 주문의 유연성으로 인해 검증되지 않은 외부 계약과의 여러 접촉 지점이 있습니다. 이는 악의적인 사용자가 주문 체결 중에 작업을 실행하기 위해 이러한 호출을 남용하고 악의적인 계약으로 조건자, 자산 또는 오라클을 가장할 수 있음을 의미합니다. 일부 영역에서는 프로젝트가 재진입으로부터 보호되지만 이러한 벡터는 서비스 거부 공격이나 감지되지 않은 스팸 주문을 유발할 수 있습니다. 1inch 팀은 프로토콜에 대해 익숙하지 않은 계약을 사용할 때 특정 문제가 나타날 수 있다는 것을 알고 있으며 주요 "블루 칩" 자산만 프로젝트에서 완전히 지원될 것이라는 의도를 밝혔습니다. 그러나 가장 인기 있는 자산이라 할지라도 각 자산에는 USDT로 전송하는 동안 수수료가 발생하거나 cToken의 성공 부울입니다.
조사 결과
여기에서 우리는 우리의 발견을 제시합니다.
심각한 심각도
없음.
높은 심각도
[H01] 일치하지 않는 데이터가 전달되었습니다. _makeCall
. OrderMixin
계약, _makeCall
기능은 자산을 전송하는 데 사용됩니다 테이커에서 메이커로 그리고 만드는 사람부터 받는 사람까지. 후자의 전송에서는 _makeCall
함수가 주문의 잘못 전달되었습니다. makerAsset
주문의 마지막 매개변수로 사용되어야 하는 경우 makerAssetData
.
결과적으로 makerAssetData
논쟁은 깨질 것이다.
이전 호출과 일관성을 유지하려면 _makeCall
프록시 기능을 완벽하게 지원하려면 업데이트를 고려하세요. order.makerAsset
에 매개 변수 order.makerAssetData
.
업데이트 : 고정됨 pull request # 57.
[H02] 부분적으로 채워진 비공개 주문은 누구나 채울 수 있습니다.
이 프로토콜을 사용하면 개인 및 공공 명령을 생성할 수 있습니다. 개인 주문의 경우, allowedSender
주문 생성 시 메이커가 지정한 주소로 주문을 완료할 수 있습니다.
그러나 OrderMixin
계약, 에 대한 검증 allowedSender
주소 범위가 잘못 지정되었습니다. 즉, 주문의 첫 번째 채우기를 처리하는 논리 내에서만 평가됩니다. 개인 주문이 부분적으로 체결된 경우 해당 주문에 대한 확인이 이루어집니다. allowedSender
주소에 더 이상 연결할 수 없으며 누구나 주문을 작성할 수 있게 됩니다.
사용자가 부분적으로 채워진 개인 주문을 처리할 수 있는지 여부에 대한 의도를 명확히 하려면 현재 동작에 대한 이유를 문서화하거나 검증하는 것을 고려하십시오. allowedSender
채우기가 시도될 때마다 유효성이 검사되도록 첫 번째 채우기 범위 외부의 주소입니다.
업데이트 : 고정됨 pull request # 58.
[H03] 악의적인 제작자는 부분 채우기를 활용하여 테이커의 자산을 훔칠 수 있습니다.
주문은 OrderMixin
계약은 부분적으로 체결될 수 있습니다. 부분 채우기를 지원하려면 프로토콜에는 스왑의 양쪽을 계산하는 방법이 필요합니다. 둘 다 getMakerAmount
와 getTakerAmount
필드는 정확한 목적을 위해 주문 제작자가 정의합니다.
주문을 작성할 때 테이커는 다음 중 하나를 제공해야 합니다. makingAmount
또는 takingAmount
가치뿐만 아니라 thresholdAmount
값. 사용할 수 있는 코드 경로는 두 가지가 있습니다. makingAmount
또는 takingAmount
제공되었습니다.
첫 번째는 makingAmount
매개변수가 정의됩니다. 그럴 수도 있지 자르다 전에, makingAmount
가치와 또한 계산 takingAmount
그것에 대한 가치. 이러한 상황에서, thresholdAmount
보장합니다 takingAmount
취한 값은 예상외로 크지는 않다.
두 번째는 takingAmount
매개변수가 정의됩니다. 그러한 경우에는 계산 makingAmount
가치, 가능성이 있는 잘림 와 다시 계산 takingAmount
그런 일이 발생하면 가치가 있습니다. 이러한 상황에서, thresholdAmount
가치는 makingAmount
반환된 값은 의외로 작지 않아.
이전에 언급한 코드 경로 중 하나에 고유한 두 가지 악용 방법이 있습니다. 이러한 악용 방법에는 악의적인 getMakerAmount
와 getTakerAmount
기능. 이러한 함수를 간단히 구현하면 다음과 동일한 동작을 갖게 됩니다. AmountCalculator
'에스 getMakerAmount
와 getTakerAmount
기능을 수행하지만 필요할 때 공격자가 제어하는 값을 반환하도록 강제하는 하드 코딩된 스위치가 있습니다.
덜 심각한 첫 번째 익스플로잇 패턴은 첫 번째 코드 경로와 관련이 있습니다. makingAmount
값이 지정됨 채우기 순서로. 악의적인 제작자는 다음을 지정하는 채우기 주문을 기다립니다. makingAmount
그것을 앞지르기 위해 mempool에 나타나게 됩니다. 그들은 제작자 측에서 1을 제외한 모든 가치를 빼낸 다음 강제로 _callGetTakerAmount
사용자가 지정한 금액을 반환합니다. thresholdAmount
가치(또는 그보다 작은 경우 허용량). 사용자의 거래가 최종적으로 완료되면 전체 거래가 교환됩니다. thresholdAmount
의 가치 takerAsset
단일 단위의 경우 makerAsset
. 이 악용은 다음에 의해 제공되는 금액으로 제한됩니다. thresholdAmount
가치나 금액 takerAsset
사용자가 허용한 LimitOrderProtocol
계약.
두 번째로 더 심각한 공격 패턴은 두 번째 코드 경로와 관련이 있습니다. takingAmount
값이 지정됨. 악의적인 제작자는 마찬가지로 다음을 지정하는 채우기 주문을 기다립니다. takingAmount
mempool에 표시할 값입니다. 그들은 거래를 미리 실행하고 강제로 makingAmount
에 의해 반환된 값 _callGetMakerAmount
두 가지보다 더 높은 기능을 수행합니다. remainingMakerAmount
그리고 thresholdAmount
. 그들은 또한 takingAmount
반환된 값: _callGetTakerAmount
금액이 될 takerAsset
에 허용되는 자산 LimitOrderProtocol
테이커에 의해. 테이커의 거래가 완료되면, 잘라내기 makingAmount
값을 매긴 다음 다시 계산하다 takingAmount
값. 그러나 이 재계산은 더 낮다고 보장되지 않으며, 이 경우 모든 금액을 받는 사람이 고갈됩니다. takerAsset
계약서에 허용한 것입니다. 이 코드 경로에서는 thresholdAmount
가치는 보장 makingAmount
너무 낮지 않아, 그래서 모든 테이커를 가져가세요 takerAsset
자산이 선택 취소되었습니다. 손실된 자금은 해당 금액에 따라 제한됩니다. takerAsset
사용자가 허용한 자산 LimitOrderProtocol
계약.
이러한 공격은 부분 주문 없이는 불가능하며, 보다 구체적으로는 악의적인 부분 주문이 필요합니다. getMakerAmount
와 getTakerAmount
구현.
의 주요 이슈는 thresholdAmount
가치 확인은 스왑의 한쪽만 커버하지만 다른 쪽은 프론트러닝을 통해 조작할 수 있다는 것입니다. 테이커가 원래 제안한 가치가 그대로 유지된다는 보장은 없습니다. 제거를 고려해보세요 makingAmount
주문이 요청한 만큼의 채우기를 지원할 수 없는 경우 코드 경로 모두에서 잘리고 되돌리기. 이렇게 함으로써, thresholdAmount
스왑의 상대방을 충분히 제한하고 악의적인 주문에서도 예상치 못한 동작을 방지하는 데 사용할 수 있습니다.
업데이트 : 고정됨 pull request # 83.
중간 정도
[M01] 동적 인수 뒤에 전달된 정적 인수
. OrderMixin
계약, getTakerAmount
와 getMakerAmount
bytes 필드는 다음의 인수로 사용됩니다. _callGetTakerAmount
와 _callGetMakerAmount
기능. 이러한 호출은 다른 쪽을 기준으로 스왑의 한쪽을 계산하는 방법을 제공하며 이를 통해 사용자는 부분적으로 주문을 체결할 수 있습니다.
XNUMXD덴탈의 getTakerAmount
/getMakerAmount
필드는 동적 변수이며 앞에 패킹됩니다. takerAmount
와 makerAmount
값 _callGetTakerAmount
와 _callGetMakerAmount
기능. 악의적인 제작자가 예상보다 많은 데이터를 제공할 가능성이 있습니다. getTakerAmount
와getMakerAmount
푸시할 필드 takerAmount
와 makerAmount
다음 함수에서 디코딩될 때 있다고 가정되는 위치를 지나간 바이트입니다. 이를 통해 메이커는 전달된 테이커 또는 메이커 금액을 전체 바이트만큼 오른쪽으로 이동할 수 있으며 추가 32바이트의 데이터가 제공되는 경우 완전히 교체할 수도 있습니다.
사용자는 이미 수동으로 검토해야 합니다. getTakerAmount
와 getMakerAmount
필드가 순서대로 나열되어 있지만 이 기술은 발견하기가 다소 어렵습니다. 또한 주목할 만한 점은 이 공격이 내부적으로 신뢰할 수 있는 공격에도 적용된다는 점입니다. getMakerAmount
와 getTakerAmount
기능. 대부분의 공격에서 합리적인 임계값을 제공하면 자금 손실을 방지할 수 있습니다.
이를 방지하려면 동적 인수 앞에 정적 인수를 인코딩하여 동적 인수에 정적 인수를 제어하는 메서드를 제공하지 않도록 하는 것이 좋습니다.
업데이트 : 고정되지 않았습니다. 1inch 팀은 다음과 같이 말했습니다.
getter 검증에 더욱 주의를 기울일 것입니다. 우리는 잠재적으로 악의적인 주문을 필터링하는 데 도움이 되는 getter의 온전성 검증을 SDK에 구현하려고 노력할 것입니다.
[M02] ERC721 주문은 조작 가능
다음을 통해 ERC20 이상을 교환할 수 있습니다. OrderMixin
IERC20과 동일한 기능 선택기를 공유하는 계약을 배포하여 transferFrom
, 그리고 해당 계약을 makerAsset
또는 takerAsset
순서대로.
범위 밖 프록시, 즉 ERC721Proxy
, ERC721ProxySafe
및 ERC1155Proxy
계약은 이 패턴을 따라 다음에 대한 지원을 제공합니다. ERC721
와 ERC1155
토큰. 프록시는 IERC20과 동일한 패턴으로 호출되어야 하므로 transferFrom
호출할 경우 서명은 다음으로 시작해야 합니다. address from
, address to
와 uint256 amount
. 프록시에 필요한 다른 모든 항목은 이후에 전달될 수 있으며 다음 순서로 정의됩니다. makerAssetData
와 takerAssetData
.
ERC1155는 자연스럽게 여러 개의 동일한 ID 토큰을 한 번에 전송할 수 있습니다. ERC1155Proxy
계약은 amount
필드. 반면에, ERC721
s에는 명확한 용도가 없습니다. amount
필드. 대체 불가능한 토큰을 나타내기 때문에 특정 tokenId는 하나만 존재하며 amount
필드는 쓸모가 없습니다. 이로 인해 두 가지 모두에 대한 구현이 ERC721Proxy
와 ERC721ProxySafe
계약은 필수를 사용합니다 amount
필드를 tokenId
대신.
이러한 과부하는 amount
매개변수는 부분적으로 채울 가능성을 생성합니다. ERC721
별도로 상장된 토큰을 할인된 가격으로 구매하기 위한 주문입니다. 예를 들어, 한 명의 사용자가 여러 명의 사용자를 보유하는 경우가 있을 수 있습니다. ERC721
동일한 계약의 양도가 허용됩니다. ERC721Proxy
계약을 맺고 별도의 지정가 주문으로 나열합니다.
지정가 주문도 제공하는 경우 getMakerAmount
와 getTakerAmount
필드를 부분적으로 채울 수 있습니다. ERC721
명령. 주문부터 amount
필드는 실제로 tokenId
, 악의적인 사용자가 부분 채우기를 배치할 수 있습니다. ERC721
더 높은 tokenId를 사용하면 makingAmount
/takingAmount
의 ERC721
이는 더 낮은 수준에 해당할 수 있습니다. tokenId
. 결과는 ERC721
더 낮은 tokenId
의 가격으로 이전될 것입니다. (higher tokenId price) * (lower tokenId's id) / (higher tokenId's id)
.
이 익스플로잇에는 몇 가지 요구 사항이 있습니다.
- 배수
ERC721
동일한 계약의 s는 둘 중 하나에서 허용됩니다.ERC721
단일 소유자에 의한 대리. - 다음 중 하나에 대한 공개 주문
ERC721
그건 최저는 아니지tokenId
허용되는 것 중. - 주문에 부분 채우기가 허용됩니다.
부분적인 가능성을 완전히 제거하려면 ERC721
채우는 경우 분리를 고려하십시오. amount
와 tokenId
인수. 인수가 분리되어 있는지 여부에 관계없이 사용자에게 이 동작을 알리고 향후 이 패턴을 방지하려면 이를 문서화하는 것도 고려하세요.
업데이트 : 고정됨 pull request # 59.
[M03] 문서화되지 않은 소수 가정
XNUMXD덴탈의 LimitOrderProtocol
계약은 상속 ChainlinkCalculator
을 통해 계약 OrderMixin
계약. 이 계약은 체인링크 오라클을 사용할 수 있도록 두 가지 기능을 제공합니다. 술어 확인 그리고 메이커 금액/테이커 금액.
그러나 계약에서는 체인링크 오라클이 보고해야 하는 소수 자릿수와 함수 매개변수에 포함되어야 하는 소수 자릿수에 대해 문서화되지 않은 가정을 합니다. 특정 시나리오에서는 이로 인해 자산 가격이 잘못 책정되거나 의도하지 않은 자금 손실이 발생하는 등 예상치 못한 행동이 발생할 수 있습니다.
더 구체적으로 말하면, 계약 전반에 걸쳐 체인링크 오라클이 소수점 이하 18자리까지 정확하게 보고할 것이라는 암묵적인 가정이 있습니다. 그러나 그렇지 않다 모든 Chainlink 오라클 이 소수점 이하 자릿수로 보고하세요. 실제로 오라클이 통화(예: USD) 기준으로 토큰 쌍을 보고하는 경우 소수점 이하 8자리만 표시됩니다. 에 대한 제한이 없기 때문에 어느 오라클을 사용할 수 있지만 보고할 소수 자릿수에 대해 암묵적인 가정을 해서는 안 됩니다.
이와 관련하여 다음과 같은 암묵적인 가정이 있습니다. amount
에 대한 매개 변수 ChainlinkCalculator
함수는 18개의 소수점 이하 자릿수를 사용하며, 이는 오해의 소지가 있는 명시적 선언과 함께 singlePrice
기능 Calculates price of token relative to ETH scaled by 1e18
. 실제로는 오라클을 사용해도 하지 소수점 이하 18자리로 보고합니다. singlePrice
함수는 소수점 이하 자릿수에 따라 크기가 조정됩니다. amount
매개변수이며 반드시 소수점 이하 18자리일 필요는 없습니다.
유사하게, doublePrice
함수는 두 개의 Chainlink 오라클이 동일한 소수점 이하 자릿수를 보고하여 함수 결과가 예상에서 벗어날 것이라고 가정합니다.
매개변수와 반환 값이 기준으로 삼아야 하는 소수 자릿수에 관한 가정을 명시적으로 문서화하는 것을 고려하세요. 또한 이러한 가정을 깨뜨리는 오라클에 의존하는 계산을 제한하거나 관련 계산에서 실제 소수 자릿수를 고려하도록 하는 것을 고려하십시오.
업데이트 : 고정됨 pull request # 75.
낮은 심각도
[L01] 명시적으로 선언되지 않은 상수
코드베이스에서 설명할 수 없는 의미로 사용되는 리터럴 값이 몇 번 발생합니다. 예를 들어:
- .
OrderMixin
계약,_remaining
매핑이 의미상 오버로드되었습니다(문제에 설명된 대로). 매핑의 의미론적 오버로딩) 부분적으로 체결된 주문에 대해 남은 자산 금액을 추적하기 위해 만큼 잘 주문이 완전히 완료된 경우. 구체적으로,0
이는 주문과 관련된 작성이 이루어지지 않았음을 의미합니다.1
이는 주문을 더 이상 채울 수 없으며 다음보다 큰 주문을 의미합니다.1
이는 잠재적으로 체결될 수 있는 주문과 관련된 남은 금액이 있음을 의미합니다. - .
ChainlinkCalculator
계약, 문자 그대로의 값1e18
에서 사용됩니다singlePrice
기능.
코드의 가독성을 높이고 리팩토링을 용이하게 하려면 모든 매직 넘버에 대해 상수를 정의하고 명확하고 설명하기 쉬운 이름을 지정하는 것이 좋습니다. 복잡한 값의 경우 계산 방법이나 선택한 이유를 설명하는 인라인 주석을 추가하는 것이 좋습니다.
업데이트 : 고정됨 pull request # 75 와 pull request # 76.
[L02] 악의적인 당사자가 허용 가능한 명령의 실행을 방해할 수 있음
XNUMXD덴탈의 OrderMixin
계약을 통해 메이커 사용자는 제출할 수 있습니다. 허용 가능한 주문 따라서 승인을 위해 별도의 트랜잭션을 거치지 않고 하나의 트랜잭션으로 실행할 수 있습니다. 또한 주문받는 사람은 자신의 허가증을 제출 동일한 목적으로 주문을 작성하는 동안.
하지만, 제조사의 허가증이 내부에 포함되어 있기 때문에 주문, 주문 작성 트랜잭션이 멤풀에 있는 동안 메이커와 테이커의 허가 모두에 액세스할 수 있습니다. 이를 통해 악의적인 사용자가 해당 허가를 받아 채우기 거래를 미리 실행하는 동안 해당 자산 계약에 대해 실행할 수 있습니다. 이 허가에는 nonce
이중 지출 공격을 방지하기 위해 프런트런에서 방금 사용된 것과 동일한 허가를 사용하려고 시도한 결과 주문 채우기 트랜잭션이 실패하게 됩니다.
보안 위험이 없고 제조업체가 새로운 주문을 생성하고 거래를 사전 승인할 수 있지만 이 공격은 확실히 허용 가능한 주문의 유용성에 영향을 미칠 수 있습니다. 실제로 동기가 부여된 공격자는 차단할 수 있습니다. 모든 이 공격으로 허용 가능한 명령. 허가증이 이미 제출되었는지 또는 허용량이 충분한지 주문이 완료되는 동안 유효성을 검사하는 것을 고려하십시오. 또한 주문 구성 중에 이러한 공격 가능성에 대해 사용자에게 알리는 것도 고려해보세요.
업데이트 : 고정되지 않았습니다. 1inch 팀은 다음과 같이 말합니다.
이전에 승인 확인이 있었지만 실패한 승인을 되돌리기 위해 허가 흐름을 단순화하기로 결정했습니다. 해당 문제를 메이커에게 알리는 방법을 고민해보겠습니다.
[L03] 중복된 코드
코드베이스 내에 중복된 코드의 인스턴스가 있습니다. 코드를 복제하면 개발 수명 주기 후반에 문제가 발생할 수 있으며 프로젝트에 오류가 발생하기 더 쉽습니다. 기능 변경 사항이 동일해야 하는 모든 코드 인스턴스에 걸쳐 복제되지 않으면 이러한 오류가 실수로 발생할 수 있습니다. 중복된 코드의 예는 다음과 같습니다.
코드를 복제하는 대신, 복제된 코드가 포함된 하나의 계약이나 라이브러리만 갖고 복제된 기능이 필요할 때마다 사용하는 것이 좋습니다.
업데이트 : 부분적으로 수정 됨 pull request # 60.
[L04] 오류가 있거나 오해의 소지가 있는 테스트 모음
테스트 스위트에는 테스트가 예상된 동작에서 벗어나는 경우가 있습니다. 예를 들어:
- XNUMXD덴탈의
ChainlinkCalculator
계약은 다음 사람에게 상속됩니다.OrderMixin
계약. 그러나 테스트 중에는AmountCalculator.arbitraryStaticCall
함수를 호출하는 데 사용됩니다.ChainlinkCalculator
외부의 독립적인 계약으로 계약합니다. 결과가 예상한 것과 같더라도 테스트는 호출을 통해 시스템의 현재 설계 및 예상 사용 사례에 따른 동작을 반영해야 합니다.ChainlinkCalculator
임의의 정적 호출을 사용하지 않고 직접 함수를 수행합니다. - 프록시 계약은 범위를 벗어났지만 ERC721 자산으로 프로토콜을 테스트할 때
ERC721Proxy
계약은 자산을 교환하는 데 사용되지 않습니다. 테스트 스위트.
테스트 스위트 자체는 이 감사 범위를 벗어나므로 테스트 스위트를 철저히 검토하여 모든 테스트가 프로토콜 사양에 따라 성공적으로 실행되는지 확인하시기 바랍니다.
업데이트 : 고정됨 pull request # 57, pull request # 59및 pull request # 61.
[L05] 이벤트 오류 및 누락
코드베이스 전반에 걸쳐 계약에 민감한 변경이 있을 때 일반적으로 이벤트가 발생합니다. 그러나 많은 이벤트에는 색인된 매개변수가 부족하거나 중요한 매개변수가 누락되어 있습니다. 예를 들어:
다음과 같이 이벤트가 부족한 민감한 작업도 있습니다.
기존 이벤트를 보다 완벽하게 색인화하고 부족한 곳에 새 매개변수를 추가하는 것을 고려해보세요. 또한 오프체인 서비스를 통해 계약 상태를 재구성하는 데 사용할 수 있는 완전한 방식으로 모든 이벤트를 내보내는 것을 고려하세요.
업데이트 : 고정되지 않았습니다. 하지만 1inch 팀은 orderRemaining
받는 매개 변수 OrderCanceled
이벤트 pull request # 62.
1inch 팀은 다음과 같이 말합니다.
우리는 프런트엔드 요구 사항을 충족하는 데 제한된 데이터 하위 집합만 필요하다는 사실을 발견했습니다. 광범위한 분석의 경우 추적을 통해 제안된 모든 필드를 사용할 수 있습니다. 을 위한
OrderRFQMixin
우리는 시장 조성자가 어떤 주문이 취소되었는지 추적하는 정교한 방법을 구축할 것으로 기대합니다.
[L06] 이벤트 발생 시 저장소 변경
. NonceManager
계약, 언제 NonceIncreased
이벤트가 발생하고, 메시지 발신자의 nonce도 증가합니다..
여러 작업을 동시에 실행하면 코드베이스를 추론하기가 더 어려워지고 오류가 발생하기 쉬우며 작업이 간과되거나 오해될 수 있습니다.
코드의 전반적인 의도성, 가독성 및 명확성을 향상하려면 이벤트를 발생시키기 전에 nonce 값을 늘리는 것이 좋습니다.
업데이트 : 고정됨 pull request # 63.
[L07] 일관되지 않은 디코딩 방법으로 인해 결과 불일치가 발생할 수 있음
모든 확장성과 유연성을 지원하기 위해 지정가 주문 프로토콜은 정기적으로 동적 바이트 데이터와 외부 계약의 임의 반환 값을 처리해야 합니다. 결과적으로 프로토콜에는 다음이 포함됩니다. ArgumentsDecoder
동적 바이트 값을 기본 데이터 유형으로 보다 효율적으로 변환하는 라이브러리입니다. 그러나 이 라이브러리를 단독으로 사용하는 것은 아니며, 경우에 따라 abi.decode
대신 사용됩니다. 또한 일부 계약에서는 abi coder v1
다른 사람들이 사용하는 동안 abi coder v2
. 전자는 다음과 더 유사하게 수행됩니다. ArgumentsDecoder
라이브러리인 반면 후자는 디코딩할 때 추가 검사를 수행합니다.
이러한 다양한 디코딩 방법론을 일관되지 않게 사용하면 코드베이스의 의도와 실제 동작 간에 미묘한 불일치가 발생할 수 있습니다.
예를 들어, simulateCalls
함수는 ArgumentsDecoder.decodeBool
함수. 만약 simulateCalls
함수는 주문의 술어 부분에서 이루어진 호출을 확인하는 데 사용되며, 그 결과는 술어 조건이 평가될 때 실제로 발생하는 것과 다를 수 있습니다. 왜냐하면 다른 디코딩 방법론이 사용되기 때문입니다.
예를 들어, 술어가 외부를 만드는 경우 staticcall
반환하는 일부 함수에 uint256
예상보다 1보다 큰 값 bool
, 반환 값이 다음과 같기 때문에 해당 호출이 되돌아갑니다. 로 디코딩 abi coder v2
'에스 abi.decode
다음과 같은 값을 허용하지 않습니다. bool
. 그러나 정확히 동일한 호출이 다음과 같이 이루어진 경우 simulateCalls
그런 다음 단순히 다음과 같이 표시됩니다. true
, 때문에 decodeBool
0보다 큰 값은 다음과 같이 처리됩니다. true
.
를하기 위해 simulateCalls
함수는 실제 조건자 호출의 동작을 완전히 반영하므로 이를 사용하도록 수정하는 것이 좋습니다. abi.decode
.
업데이트 : 고정됨 pull request # 82.
[L08] 입력 유효성 검사 부족
XNUMXD덴탈의 fillOrderToWithPermit
와 fillOrderTo
의 기능 OrderMixin
계약은 물론이고 fillOrderRFQToWithPermit
와 fillOrderRFQTo
의 기능 OrderRFQMixin
계약, 검증하지 마세요 target
주소 매개변수.
이로 인해 사용자가 실수로 0 주소를 전달할 수 있으며 결과적으로 주문을 완료한 후 받게 될 자산이 잠길 수 있습니다.
사용자가 실수로 자금을 잠그는 일이 없도록 하려면 target
주소는 인용된 함수의 0 주소와 동일하지 않습니다.
업데이트 : 고정됨 pull request # 78.
[L09] 낮은 단위 테스트 범위
전체 프로젝트의 단위 테스트 적용 범위는 약 75%이며 일부 계약의 적용 범위는 특히 낮습니다.
새로운 기능을 리팩토링하고 개발할 때 코드를 검증하고 회귀를 방지하기 위한 단위 테스트의 중요성을 고려하여 단위 테스트 적용 범위를 최소 95%로 크게 늘리고 예상치 못한 상황도 포함하는 극단적인 사례를 포함하는 것이 좋습니다.
업데이트 : 고정되지 않았습니다.
[L10] 오해의 소지가 있거나 불완전한 인라인 문서
코드베이스 전체에서 오해의 소지가 있거나 불완전한 인라인 문서의 몇 가지 사례가 확인되었으며 수정되어야 합니다.
다음은 오해의 소지가 있는 인라인 문서의 예입니다.
- .
ChainlinkCalculator
계약,singlePrice
기능 NatSpec@notice
태그 그것을 말한다Calculates price of token relative to ETH scaled by 1e18
, 그러나 실제로 그 결과는 다음과 같습니다. 가치 ofamount
토큰 크기 조정1e18
, 여기서 오라클은 ETH를 기준으로 보고하지 않을 수 있습니다(예를 들어 ETH를 포함하지 않는 쌍의 경우). - .
OrderRFQMixin
계약,invalidatorForOrderRFQ
기능 NatSpec@return
태그 해당 무효화 비트가 설정되도록 견적이 채워지지 않았을 수 있기 때문에 오해의 소지가 있습니다. 주문이 취소되었을 수도 있습니다. - 온라인 147, 165및 188 of
OrderMixin.sol
, NatSpec@param
태그가 문법에 맞지 않습니다. - 온라인 20 of
ERC1155Proxy.sol
Walk Through California 프로그램,@notice
태그에는 계산된 해시가func_733NCGU
함수가 있어야 할 곳func_301JL5R
대신 기능.
다음은 불완전한 인라인 문서의 경우입니다.
- 기능
AmountCalculator
계약은 어떤 매개변수도 설명하지 않습니다. - .
ChainlinkCalculator
계약,singlePrice
와doublePrice
함수는 모든 매개변수를 설명하지 않습니다. - .
ImmutableOwner
계약에 따라 공용 변수 및 수정자에는 NatSpec이 없습니다. - .
InteractiveNotificationReceiver
계약,notifyFillOrder
함수는 어떤 매개변수도 설명하지 않습니다. - .
LimitOrderProtocol
계약,DOMAIN_SEPARATOR
함수에는 NatSpec이 없습니다. - 이벤트 및 매핑
NonceManager
NatSpec이 없습니다. - .
OrderRFQMixin
계약,cancelOrderRFQ*
함수는 반환 값을 설명하지 않습니다. - .
OrderMixin
계약에 따라 여러 기능에 완전한 NatSpec이 부족합니다. - 온라인 168 of
OrderMixin.sol
그리고 온라인으로 71 ofOrderRFQMixin.sol
, 가 누락되었습니다.@dev
꼬리표. - 기능
PredicateHelper
계약은 모든 매개변수를 설명하지 않습니다.
명확한 인라인 문서는 코드의 의도를 개략적으로 설명하는 데 필수적입니다. 인라인 문서와 구현 간의 불일치로 인해 시스템이 어떻게 작동할지에 대한 심각한 오해가 발생할 수 있습니다. 개발자, 사용자, 감사자 모두의 혼란을 피하기 위해 이러한 오류를 수정하는 것이 좋습니다.
업데이트 : 부분적으로 수정되었습니다. 다음에서 다루어진 오해의 소지가 있는 문서 pull request # 75 와 pull request # 77.
1inch 팀은 다음과 같이 말합니다.
오해의 소지가 있는 문서를 수정했습니다. 문서 완성은 나중에 완료됩니다.
[L11] Hook 사용 시 DoS 명령 가능
XNUMXD덴탈의 OrderMixin
계약은 성공 조건이 있을 수 있는 일반 오프체인 스왑 주문을 작성하는 기능을 구현합니다. 주문이 완료되는 동안 주문이 완료될 수 있습니다. 미리 정의된 "술어" 조건을 확인하세요. 실행을 계속하기 전에.
그러나 이러한 조건은 임의 계약의 논리를 표적으로 삼을 수 있기 때문에 악의적인 제작자는 주문이 올바르게 작동하고 오프체인에서 확인할 때 유효하다고 믿도록 테이커를 속일 수 있지만 동일한 주문을 작성하려고 하면 실패할 수 있습니다. 온체인. 조건자 동작의 이러한 변경은 조건자가 의존하는 일부 변수 상태를 미리 실행하거나 전송된 가스를 검사하거나 심지어 호출에 관련된 주소를 검사하거나 다른 논리를 통해 이루어질 수 있습니다.
게다가, 제작자가 정의한 경우 교환 중 상호 작용Walk Through California 프로그램, interactionTarget
계약은 성공적인 주문 이행을 방지하기 위해 자체적으로 되돌리거나 허용을 취소할 수 있으며, 이는 본질적으로 악의적인 조건과 동일한 결과를 초래할 수 있습니다.
자산이 위험에 처하지는 않더라도 유리한 주문을 찾는 사용자나 봇은 표면적으로는 합법적인 것처럼 보일 수 있는 이러한 종류의 스팸 주문을 식별해야 하는 부담이 커질 것입니다. 이러한 종류의 주문을 식별하지 못하는 경우 가스 비용이 낭비됩니다. 스팸 주문량을 줄이려면 이러한 후크에 사용 가능한 대상을 제한하는 것이 좋습니다. 또한 사용자가 주문을 이행하기 전에 이러한 가능성에 대해 경고하는 것이 좋습니다.
업데이트 : 고정되지 않았습니다. 1inch 팀은 다음과 같이 말합니다.
우리는 이를 백엔드에서 처리하고 가능한 수용자에게 문제에 대해 알리는 방법에 대해 생각할 것입니다.
[L12] 반올림은 다음 경우에 적합하지 않을 수 있습니다. taker
. OrderMixin
와 OrderRFQMixin
계약, 주문이 체결되고 테이커가 주문만 제공하는 경우 makingAmount
or takingAmount
금액인 경우, 프로토콜은 스왑의 대응 금액을 계산하려고 시도합니다.
이러한 계산에는 두 가지 문제가 있습니다. 첫 번째는 양 매개변수가 사용해야 하는 소수 자릿수를 제한하는 문서나 논리가 없다는 것입니다. 문서화되지 않은 소수 가정 발행물.
두 번째 문제는 이러한 계산 과정에서 프로토콜이 제작자에게 유리하게 작용한다는 것입니다. 반올림 문제는 암시적 소수 가정이 깨지면 크게 악화될 수 있지만, 모든 것이 예상된 항에 있는 경우에도 작고 이상한 금액으로 반올림이 발생합니다.
응시자가 최소 금액을 지정할 수 있도록 허용하는 것을 고려하십시오. makerAsset
최대 금액과 함께 받고자 하는 자산 takerAsset
교환할 의향이 있는 자산이므로 반올림 허용이 더욱 명확해집니다.
업데이트 : 고정되지 않았습니다. 1inch 팀은 다음과 같이 말합니다.
임계값은 테이커를 보호하기에 충분해야 합니다.
[L13] 매개변수가 부족한 경우 모순되는 순서 처리
. OrderMixin
계약, fillOrderTo
함수는 내부 호출을 수행합니다. _callGetMakerAmount
와 _callGetTakerAmount
채우기가 시도될 때마다 기능하며 makingAmount
또는 takingAmount
매개변수가 각각 0이거나 makingAmount
값이 다음보다 큽니다. remainingMakerAmount
값.
XNUMXD덴탈의 _callGetMakerAmount
와 _callGetTakerAmount
주문이 생성되지 않은 경우 통화가 취소될 수 있습니다. getMakerAmount
or getTakerAmount
매개변수에 따라 부분 채우기가 실행됩니다.
An 인라인 주석 옆에 _callGetMakerAmount
및 인라인 주석 옆에 _callGetTakerAmount
주문이 다음과 같이 생성되지 않은 경우 "전체 채우기만 허용됩니다"라고 주장 getMakerAmount
or getTakerAmount
매개 변수를 설정합니다.
그러나 이것이 적용되지 않는 코드 경로도 있습니다. 해당 경로는 length
둘 다 getMakerAmount
와 getTakerAmount
매개 변수를 설정합니다.
특히, 때 taker
지정 takerAmount
다음과 같은 주문 값만 있습니다. getMakerAmount
, 해당 호출이 아닌 이상 getMakerAmount
다음보다 큰 금액을 반환합니다. remainingMakerAmount
, 인라인 문서와 반대로 부분 채우기가 실행될 수 있습니다.
이로 인해 해당 코드 경로의 의도가 불분명해집니다. 이것이 예상되는 동작이라면 인라인 문서를 보다 명확하게 수정하는 것이 좋습니다. 이것이 의도하지 않은 동작인 경우 항상 두 개의 길이를 모두 확인하는 것이 좋습니다. getMakerAmount
그리고 getTakerAmount
구현 시 인라인 문서에 설명된 동작을 강화하도록 매개변수를 동시에 사용합니다.
업데이트 : 고정됨 pull request # 79.
[L14] 더 이상 사용되지 않는 Chainlink 호출 사용
XNUMXD덴탈의 ChainlinkCalculator
계약은 Chainlink 오라클에 쿼리하는 데 사용됩니다. 이는 해당 고객에게 전화를 걸어 이를 수행합니다. latestTimestamp
와 latestAnswer
행동 양식, 둘 다 더 이상 사용되지 않습니다.. 실제로 해당 메소드는 Chainlink 수집기의 API에 더 이상 존재하지 않습니다. 버전 3부터.
향후 Chainlink 오라클과의 비호환 가능성을 방지하려면 다음을 사용하는 것이 좋습니다. latestRoundData
대신 방법.
업데이트 : 고정됨 pull request # 67.
참고 및 추가 정보
[N01] 인터페이스를 가져오지 않음
XNUMXD덴탈의 AggregatorInterface
인터페이스는 다음에서 복사된 코드의 하위 집합인 것 같습니다. ChainLink
님의 공개 코드 저장소. 전체 인터페이스는 다음에 포함되어 있습니다. ChainLink
님의 계약 npm 패키지.
가능하다면 인터페이스 불일치 및 그에 따른 문제의 가능성을 줄이려면 다른 프로젝트의 인터페이스를 다시 정의하거나 다시 작성하는 대신 공식 npm 패키지를 통해 설치된 인터페이스를 사용하는 것이 좋습니다.
업데이트 : 고정됨 pull request # 66.
[N02] 더 이상 사용되지 않는 프로젝트 종속성
설치하는 동안 프로젝트의 종속성, NPM은 설치된 패키지 중 하나가 Highlight
, "앞으로 더 이상 지원되지 않거나 보안 업데이트를 받지 않을 것입니다."
이 패키지가 보안 위험을 유발할 가능성은 거의 없지만 이 패키지를 사용하는 종속성을 유지 관리 버전으로 업그레이드하는 것이 좋습니다.
업데이트 : 고정됨 pull request # 64.
[N03] 뷰 메소드에 대한 외부 호출은 정적 호출이 아닙니다.
대부분의 코드베이스에서 프로토콜은 OpenZeppelin을 통해 명시적으로 외부 호출을 수행합니다. functionStaticCall
예상되지 않거나 바람직하지 않은 경우 상태 변경 가능성을 제한하는 방법입니다. 그러나 ChainlinkCalculator
외부 전화만 걸겠다는 의도에도 불구하고 계약을 체결했습니다. view
Chainlink 오라클의 메소드, 외부 호출 singlePrice
와 doublePrice
함수는 명시적으로 만들어지지 않습니다. staticcall
s.
이로 인해 발생하는 즉각적인 보안 문제는 확인되지 않았지만 공격 표면을 줄이고 일관성을 개선하며 의도를 명확히 하려면 명시적 사용을 고려하세요. staticcall
s, 모든 외부 통화에 대해 view
기능 ChainlinkCalculator
계약.
업데이트 : 고정되지 않았습니다. 1inch 팀은 다음과 같이 말합니다.
우리는 구문 복잡도가 일관성 향상을 무효화한다고 생각합니다.
[N04] 잘못된 주문으로 조기 실패하지 않음
. OrderMixin
계약, fillOrderTo
함수는 주문이 이전에 제출되지 않은 경우 특수 조건을 처리합니다(remainingMakerAmount == 0
), 그러나 주문이 더 이상 유효하지 않은 경우 조건을 명시적으로 처리하지 않습니다(remainingMakerAmount == 1
).
후자의 경우, 기능은 결국 원래대로 돌아가지만, 적지 않은 양의 가스를 연소한 후에만 가능합니다. 의도를 명확히 하고, 가독성을 높이고, 가스 사용량을 줄이려면 함수 시작 부분에서 잘못된 순서 시나리오를 명시적으로 처리하는 것이 좋습니다.
업데이트 : 고정됨 pull request # 68.
[N05] 초록으로 표시되지 않은 도우미 계약
Solidity에서 키워드는 abstract
그 자체로는 기능적 계약이 아니거나 기능적 계약으로 사용할 의도가 없는 계약에 사용됩니다. 대신에, abstract
계약은 독립형 기능 계약을 생성하기 위해 시스템의 다른 계약에 의해 상속됩니다.
코드베이스 전체에는 자체적으로 배포할 의도가 없음에도 불구하고 추상으로 표시되지 않은 도우미 계약의 예가 있습니다. 예를 들어, AmountCalculator
, ChainlinkCalculator
, ImmutableOwner
, NonceManager
및 PredicateHelper
계약은 모두 계약 상속에 사용되는 기본 기능 세트로 구성됩니다.
도우미 계약을 다음으로 표시하는 것을 고려하세요. abstract
이를 상속하는 계약에 기능을 추가하기 위해서만 설계되었음을 명확하게 나타냅니다.
업데이트 : 고정되지 않았습니다. 1inch 팀은 다음과 같이 말합니다.
해당 도우미는 별도로 배포할 수 있습니다. 가스 절약을 위해서만 상속됩니다.
[N06] 일관되지 않은 기능 순서
코드베이스 전체에서 함수 순서는 일반적으로 다음을 따릅니다. Solidity 스타일 가이드에서 권장되는 순서어느입니다 : constructor
, fallback
, external
, public
, internal
, private
.
그러나 OrderMixin
계약, public
checkPredicate
함수가 스타일 가이드에서 벗어나서 external
기능.
프로젝트의 전반적인 가독성을 높이려면 Solidity 스타일 가이드에서 권장하는 대로 코드베이스 전체에서 함수 순서 표준화를 고려하세요.
업데이트 : 고정됨 pull request # 69.
[N07] 일관되지 않은 주문 처리 흐름
XNUMXD덴탈의 OrderMixin
와 RFQOrderMixin
두 계약 모두 서명된 주문의 작성을 처리하지만 두 계약 간의 일반적인 주문 흐름은 일관되지 않습니다.
OrderMixin
'에스 fillOrderTo
함수는 다음과 같은 일반적인 흐름(의사 코드)을 따릅니다.
if ((takingAmount == 0) == (makingAmount == 0))
else if (takingAmount == 0)
else (handle makingAmount == 0) THEN swapTokens
반면 RFQOrderMixin
유사하다 fillOrderRFQTo
함수는 다음 흐름(의사 코드)을 따릅니다.
if (takingAmount == 0 && makingAmount == 0)
else if (takingAmount == 0)
else if (makingAmount == 0)
else revert THEN swapTokens
이 두 함수 각각의 첫 번째 조건이 왜 다른지 또는 왜 다른지에 대한 통찰력이 문서에 없습니다. takingAmount
와 makingAmount
후자의 함수에서는 둘 다 0일 수 없습니다. 또한, 두 가지 모두 makingAmount
및 takingAmount
제공되는 것이 추론하기가 훨씬 쉽습니다. fillOrderRFQTo
함수는 최종 단계에서 명확하게 처리되므로 else
블록.
의도를 명확히 하고 코드의 전반적인 가독성을 높이려면 두 계약의 일반 주문 흐름을 표준화하거나 차이점이 존재하는 이유를 명시적으로 문서화하는 것을 고려하세요.
업데이트 : 고정되지 않았습니다. 1inch 팀은 다음과 같이 말합니다.
이는 지정가 주문의 맞춤형 가격 책정 기능 때문입니다. 부터
getMakerAmount
잠재적으로 다음과 크게 다를 수 있습니다.getTakerAmount
, 우리는 getter가 다를 경우 테이커를 혼란스럽게 할 수 있으므로 테이커에 대한 기본 옵션을 설정하지 않는 것이 더 낫다고 생각했습니다.
[N08] 오류 메시지의 형식이 일관되지 않거나 오해의 소지가 있습니다.
코드베이스 전반에 걸쳐 require
와 revert
거래 실패를 초래하는 특정 조건을 사용자에게 알리기 위한 오류 메시지의 형식이 일관되지 않은 것으로 밝혀졌습니다.
예를 들어, 라인의 각 오류 메시지는 85 OrderMixin.sol
, 16 ERC721ProxySafe.sol
및 26 Permitable.sol
다른 스타일을 사용하십시오.
또한 일부 오류 메시지는 오해의 소지가 있습니다.
오류 메시지는 사용자에게 실패 조건을 알리기 위한 것이므로 시스템과 상호 작용하기 위해 적절한 수정이 이루어질 수 있도록 충분한 정보를 제공해야 합니다. 정보가 없는 오류 메시지는 전반적인 사용자 경험을 크게 손상시켜 시스템 품질을 저하시킵니다. 더욱이 일관되지 않은 형식의 오류 메시지는 불필요한 혼란을 야기할 수 있습니다. 그러므로 전체 코드베이스를 검토하여 모든 사항을 확인하는 것이 좋습니다. require
와 revert
명령문에는 일관되고 정확하며 유익하고 사용자 친화적인 오류 메시지가 있습니다.
업데이트 : 부분적으로 수정 됨 pull request # 81.
[N09] 명명된 반환 변수의 일관성 없는 사용
명명된 반환 변수의 일관성 없는 사용이 있습니다. OrderMixin
계약. 일부 기능 명명된 변수를 반환, 기타 명시적인 값을 반환, 다른 사람 명명된 반환 변수를 선언하지만 재정의합니다. 명시적인 return 문을 사용합니다.
명명된 모든 반환 변수를 제거하고, 명시적으로 지역 변수로 선언하고, 적절한 경우 필요한 반환 문을 추가하여 코드베이스 전체에서 반환 값에 대한 일관된 접근 방식을 채택하는 것을 고려하십시오. 이렇게 하면 코드의 명확성과 가독성이 모두 향상되고 향후 코드 리팩터링 중에 회귀를 줄이는 데 도움이 될 수도 있습니다.
업데이트 : 고정됨 pull request # 73.
[N10] 주문의 해시 계산이 API에 공개되지 않습니다.
XNUMXD덴탈의 external
기능 remaining
, remainingRaw
와 remainingsRaw
모두 성공적인 작업을 위한 주문 해시를 기대합니다.
그러나 도우미 기능 _hash
주문의 해시를 반환하는 에는 private
시계. 이는 사용자가 주문 해시를 얻으려면 주문 및 도메인 문자열의 일부를 수동으로 압축해야 함을 의미합니다.
주문 해시를 계산할 때 발생할 수 있는 실수를 방지하고 사용자에게 주문의 각 해시를 생성하는 방법을 제공하려면 _hash
기능 public
이름을 다음으로 리팩토링합니다. hash
나머지 코드와 일관성을 유지합니다.
업데이트 : 고정됨 pull request # 74.
[N11] 매핑의 의미론적 오버로딩
XNUMXD덴탈의 _remaining
매핑 OrderMixin
계약은 주문 상태와 해당 주문에 사용할 수 있는 남은 자산 금액을 추적하기 위해 의미상 오버로드됩니다.
취할 수 있는 세 가지 상태는 다음과 같습니다.
0
: 주문 해시가 아직 확인되지 않았습니다.1
: 주문이 취소되었거나 완전히 체결되었습니다.- 모든 품종
uint
보다 큰1
: 남은makerAmount
플러스 주문 시 충전 가능1
.
이 의미론적 오버로딩에는 실행 중에 이 값을 래핑하고 언래핑해야 합니다. lookup
, cancellation
, initialization
및 storage
.
의미론적 오버로딩과 이를 활성화하는 데 필요한 논리는 오류가 발생하기 쉽고 코드베이스를 이해하고 추론하기 어렵게 만들 수 있으며 향후 코드 업데이트에서 회귀의 문을 열 수도 있습니다.
코드의 가독성을 높이려면 별도의 매핑에서 주문 완료 상태를 추적하는 것이 좋습니다.
업데이트 : 고정되지 않았습니다. 1inch 팀은 수정으로 인해 가스 비용이 증가할 것이라고 언급했습니다. fillOrder
기능.
[N12] 허가가 있는 주문은 임의 계약 호출을 허용합니다.
XNUMXD덴탈의 OrderMixin
계약은 상속 Permitable
이를 수락하는 자산으로 단일 거래 주문을 작성하는 것을 허용하는 계약 permit
수당 수정을 요청합니다.
그러나, 에게 전화를 겁니다 Permitable
계약 대상이 허용 가능한 자산인지 또는 자산인지 여부를 검증하지 마십시오. 이로 인해 악의적인 사용자가 주문 작성이 완료되기 전에 다른 호출을 실행할 수 있는 임의 계약의 주소를 전달할 수 있습니다.
비록 계약은 재진입으로부터 보호됨, 공격 표면을 줄이고 실행 중에 외부 계약 호출을 방지하는 것이 항상 권장됩니다. 허가와 관련된 자산을 주문과 관련된 자산으로 제한하거나 프로토콜에 대한 자산 화이트리스트로 제한하는 것을 고려하십시오.
업데이트 : 고정되지 않았습니다. 1inch 팀은 다음과 같이 말합니다.
OrderMixin
실제로 실제 토큰에 대한 정보가 없습니다.makerAsset
와takerAsset
때로는 프록시 또는 기타 중간 계약이며 실제 토큰에 대한 정보는 임의의 바이트에 저장됩니다. 따라서 어떤 자산을 제한할 수 있는 실행 가능한 방법이 없습니다.permit
호출됩니다.
[N13] solhint
다시 활성화되지 않음
코드베이스 전체에 걸쳐 몇 가지가 있습니다. solhint-disable
진술, 특히 온라인에 있는 진술 23 그리고 온라인으로 41 of RevertReasonParser.sol
, 다음으로 끝나지 않습니다. solhint-enable
.
명시성을 선호하고 비활성화할 때 최대한 제한합니다. solhint
, 사용 고려 solhint-disable-line
or solhint-disable-next-line
대신 line과 유사합니다. 16 같은 파일의.
업데이트 : 고정됨 pull request # 72.
[N14] 오타
코드베이스에는 다음과 같은 오타가 포함되어 있습니다.
추가적으로 프로젝트의 README
(이 감사 범위 외)에는 다음과 같은 오타가 포함되어 있습니다.
코드 가독성을 높이려면 이러한 오타를 수정하는 것이 좋습니다.
업데이트 : 고정됨 pull request # 71 와 pull request # 77.
[N15] 사용 uint
대신 uint256
명확성을 높이기 위해 모든 인스턴스는 uint
다음과 같이 선언되어야 한다. uint256
. 특히, for
라인의 루프 98 와 119 of OrderMixin.sol
그리고 선 16 와 30 of PredicateHelper.sol
.
업데이트 : 고정됨 pull request # 70.
결론
심각도가 높은 문제 3개가 발견되었습니다. 모범 사례를 따르고 잠재적인 공격 표면을 줄이기 위해 일부 변경 사항이 제안되었습니다.
- &
- 7
- 소개
- ACCESS
- 에 따르면
- 계정
- 가로질러
- 행동
- 행위
- 추가
- 주소
- 이점
- All
- 허용
- 이미
- 이기는하지만
- 금액
- 분석
- API를
- 접근
- 인수
- 약
- 유산
- 자산
- 회계 감사
- 백엔드
- 처음
- 존재
- BEST
- 모범 사례
- 비트
- 봇
- 빌드
- 전화
- 한
- 가지 경우
- 원인
- 체인 링크
- 이전 단계로 돌아가기
- 확인
- 확인하는 것이 좋다.
- 암호
- 복잡한
- 조건
- 혼동
- 구조
- 이 포함되어 있습니다
- 계약
- 계약
- 수정
- 비용
- 수
- 두
- 창조자
- 환율
- Current
- 데이터
- 거래
- 서비스 거부
- 배치
- 디자인
- 개발자
- 개발
- DID
- 다르다
- 다른
- 도메인
- 더블
- 동적
- 초기의
- Edge
- 격려
- 특히
- ETH
- 이벤트
- 이벤트
- 모두
- 예
- 교환
- 기대하는
- 경험
- 공적
- 특징
- Fields
- 최종적으로
- 먼저,
- 수정
- 유연성
- 흐름
- 따라
- 발견
- 가득 찬
- 기능
- 자금
- 미래
- 경기
- 가스
- 일반
- 기부
- 큰
- 안내
- 처리
- 해시
- 해싱
- 데
- 도움
- 높은
- 고도로
- 방법
- HTTPS
- 잡종
- 확인
- 영향
- 구현
- 중대한
- 가져 오기
- 포함
- 포함
- 증가
- 증가
- 정보
- 정보
- 인프라
- 통찰력
- 의지
- 관심
- 인터페이스
- 참여
- 문제
- IT
- 넓은
- 큰
- 리드
- 지도
- 도서관
- 제한된
- 라인
- 유동성
- 상장 된
- 기울기
- 지방의
- 보고
- 조회
- 주요한
- 메이커
- 유튜브 영상을 만드는 것은
- 시장
- 멤풀
- 거울
- 모델
- 가장
- 가장 인기 많은
- 즉
- 새로운 기능
- 비 대체 가능
- 대체 할 수없는 토큰
- 공무원
- 열 수
- 행정부
- 선택권
- 신탁
- 주문
- 명령
- 기타
- 소유자
- 무늬
- 인기 문서
- 제시
- 방지
- 가격
- 가격
- 사설
- 방법
- 프로젝트
- 프로젝트
- 보호
- 프로토콜
- 제공
- 제공
- 대리
- 공개
- 게시
- 매수
- 품질
- 모집
- 현실
- 감소
- 신뢰
- 신고
- 보고서
- 저장소
- 요구조건 니즈
- REST
- 결과
- 반품
- 리뷰
- 위험
- 회진
- 달리기
- SDK
- 보안
- 서비스
- 세트
- 공유
- 공유
- 변화
- 비슷한
- 단순, 간단, 편리
- 작은
- 스마트 한
- 스마트 계약
- So
- solidity
- 스팸
- 구체적으로
- 지출
- Spot
- 스타트
- 주 정부
- 성명서
- 미국
- Status
- 저장
- 스타일
- 제출
- 성공
- 성공한
- 성공적으로
- SUPPORT
- 지원
- 표면
- 스위치
- 체계
- 목표
- test
- 지원
- 테스트
- 을 통하여
- 도처에
- 시간
- 함께
- 토큰
- 토큰
- 선로
- 추적
- 거래
- 믿어
- 유일한
- 업데이트
- us
- 유용성
- USD
- USDT
- 사용자
- 유틸리티
- 가치
- 관측
- 가시성
- 기다리다
- 뭐
- 허용 된 사이트 목록
- 이내
- 없이
- 작업
- 가치
- 제로