2021 년 12 월 6 일
개요
XNUMXD덴탈의 계정 팀은 공통 거버넌스 계약을 개선하고 더 많은 유연성을 제공하기 위한 최종 목표를 가지고 일련의 계약을 검토하고 감사하도록 요청했습니다. 우리는 코드를 살펴보고 이제 결과를 게시합니다.
시스템 개요
이 시스템은 세 가지 주요 계약에 의존합니다.
- A
SafeGuard
계약서 양식입니다. 이 계약은admin
의Timelock
계약을 체결하고 위에 모듈식 역할 레이어를 추가합니다.Timelock
의 행동. 이 계약은 제안서 상태(대기열, 취소 및 실행)에 대해 별도의 책임과 액세스 권한을 갖는 여러 역할을 정의합니다. - A
SafeGuardFactory
계약이 새로운 것을 배포합니다.SafeGuard
그리고 이에 상응하는 새로운Timelock
계약. 그런 다음 내부에 타임록 주소를 설정합니다.SafeGuard
계약을 체결하고 신규 등록을 합니다.SafeGuard
로Registry
. - A
Registry
배포된 목록을 보유하는SafeGuards
해당하는 버전 번호.
XNUMXD덴탈의 SafeGuardFactory
기본적으로 SafeGuard
호출될 때마다. ㅏ SafeGuard
둘러싸는 것입니다 Timelock
운영 각 작업에 대해 서로 다른 분리된 역할을 추가합니다. 역할은 OpenZeppelin의 사용을 통해 구성됩니다. AccessControlEnumerable
여러 주소가 동일한 공유 역할을 가질 수 있는 다중 서명 지갑 및 비즈니스 사용 사례에 더 많은 유연성과 호환성을 제공하는 계약입니다.
업데이트 : . PR # 10, Tally 팀은 Registry
계약. 배포된 보호 장치 목록은 이제 SafeGuardFactory
계약, 에서 safeGuards
열거 가능한 세트와 해당 버전이 safeGuardVersion
매핑.
역할
XNUMXD덴탈의 SafeGuard
계약은 다음 역할을 정의합니다.
업데이트 : XNUMXD덴탈의 CREATOR_ROLE
L02 문제에 대한 수정으로 역할이 제거되었습니다.
범위
커밋을 감사했습니다. b2c63a9dfc4090be13320d999e7c6c1d842625d3
의 safeguard
저장소. 범위에는 스마트 계약이 포함됩니다. contracts
예배 규칙서. 그러나, 그 mocks
디렉토리는 범위를 벗어난 것으로 간주되었습니다.
가정
시스템은 업그레이드할 수 없습니다. 그만큼 Registry
주소가 설정되었습니다 생성자에서 의 SafeGuardFactory
그리고 각각 SafeGuard
가질 수 있습니다 Timelock
한 번만 설정. 즉, 새로운 경우 Registry
새로 배포되었습니다. SafeGuardFactory
배포도 해야 합니다. 만약 Timelock
구현 변경, 이전 SafeGuard
s는 더 이상 사용되지 않게 되며 새로운 것을 배포해야 합니다.
더욱이, 시스템은 다음의 구현에 크게 의존합니다. Timelock
계약 이는 이번 감사 범위를 벗어난 것으로 간주되었습니다. 팀은 아직 구현을 마무리하지 않았습니다. Timelock
계약. 이 보고서를 작성할 당시, 화합물의 시간 잠금 구현 프로젝트에 사용되고 있습니다.
코드베이스는 일주일 동안 두 명의 감사자에 의해 감사되었으며 여기에 우리의 결과가 나와 있습니다.
심각한 심각도
없음.
높은 심각도
[H01] ETH는 내부에 잠길 수 있습니다. Timelock
계약
XNUMXD덴탈의 Tally
팀은 원래 다음을 기반으로 구현을 기반으로 했습니다. GovernorBravoDelegate
복합 계약.
이번 감사 과정에서, Tally
팀 발견 제한 ETH가 직접적으로 전송된 컴파운드의 거버너에서 Timelock
거버넌스 제안에서 사용할 수 없으며 영구적으로 중단되지는 않지만 검색하려면 정교한 해결 방법이 필요합니다.
이는 거버너 구현 시 제안의 모든 값이 다음과 같이 첨부되어야 하기 때문입니다. msg.value
어떤 방식으로든 사용하지 않고 실행을 트리거하는 계정에 의해 Timelock
ETH 자금.
XNUMXD덴탈의 동일한 문제가 나중에 SafeGuard
이행 팀에서도 해당 문제를 인지하고 있으며 현재 해결 중입니다.
문제를 해결하는 동안 다음 접근 방식을 사용해 보세요. 동일한 문제에 대해 OpenZeppelin 라이브러리에서 채택됨.
업데이트 : 커밋에서 수정됨 7337db227edda83533be586135d96ddac4f5bf29
.
[H02] SafeGuardFactory가 동결될 수 있습니다.
XNUMXD덴탈의 Registry
계약은 모든 사항을 추적하기 위한 것입니다. SafeGuards
그 SafeGuardFactory
생산하다. 그것은 외부 register
기능 이 목적으로 사용됩니다.
동시에, SafeGuardFactory
생성자에는 로컬 할당이 있습니다. registry
값을 입력 값으로. 값을 변경할 가능성은 없습니다. registry
변수가 있으므로 이러한 이유로 새로운 Registry
배포되면 새 공장도 배포해야 합니다.
XNUMXD덴탈의 SafeGuardFactory
가 createSafeGuard
기능, 첫 번째 담당 새로운 배포 SafeGuard
다음, 새로운 Timelock
주소와 함께 SafeGuard
as admin
, 그런 다음 설정 timelock
변수의 SafeGuard
계약하고 드디어 등록 SafeGuard
레지스트리에서.
문제는 createSafeGuard
새 주소의 결정론적 주소를 직접 등록할 수 있는 공격자에 의해 강제로 실패할 수 있습니다. SafeGuard
생성되기 전에. 계약할 때마다 ~을 창조하다 new
예, 해당 nonce가 증가하고 계약의 새 인스턴스가 배포될 주소는 원래 계약 주소와 해당 nonce에 의해 결정될 수 있습니다. 따라서 공격자는 새 주소가 있는 많은 주소를 미리 계산할 수 있습니다. SafeGuards
배포되고 해당 주소를 Registry
전화하여 register
기능. 이로 인해 다음이 호출됩니다. createSafeGuard
에 돌아가는 것 이후 Registry
이미 주소가 포함되어 있습니다.
외부 행위자가 공개적으로 전화하는 것을 방지하려면 Register
계약을 체결한 경우에는 액세스를 제한하는 것을 고려하세요. register
전화를 단독으로 수락하는 기능 SafeGuardFactory
.
업데이트 : 고정됨 PR # 10. Tally 팀은 다음을 제거했습니다. Registry
계약.
중간 정도
없음.
낮은 심각도
[L01] 주석 처리된 코드
XNUMXD덴탈의 Registry
계약에는 다음이 포함됩니다 주석 처리된 코드 줄. 가독성을 높이려면 코드베이스에서 제거하는 것이 좋습니다.
업데이트 : 고정됨 PR # 10 그리고 커밋 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L02] SafeGuard 관리자는 모든 주소에 작성자 역할을 할당할 수 있습니다.
XNUMXD덴탈의 SafeGuard
계약은 다음의 역할을 정의합니다. CREATOR_ROLE
이름에서 알 수 있듯이 보호 장치 작성자에게 할당됩니다.
그러나 호출하여 전에, grantRole
기능 AccessControlEnumerable
계약 OpenZeppelin 계약 라이브러리에서 관리자는 모든 주소에 이 역할을 부여할 수 있습니다. 이로 인해 혼란이 발생할 수 있습니다. 의 창조자 SafeGuard
오직 SafeGuardFactory
.
코드베이스 전체에서 이 역할은 사용자가 setTimelock
기능 의 SafeGuard
계약. 설계상 시스템은 다음을 보장합니다. setTimelock
기능 한 번만 호출할 수 있습니다.에서 이내 SafeGuardFactory
계약.
제거를 고려해보세요. CREATOR_ROLE
의 역할 SafeGuard
계약하고 사용하는 onlyOwner
변화 FBI 증오 범죄 보고서 setTimelock
기능.
업데이트 : 고정됨 PR # 10.
[L03] 잘못된 인터페이스 정의 및 구현
XNUMXD덴탈의 ISafeGuard
인터페이스는 정의하지 않습니다 queueTransactionWithDescription
기능 에서 구현 SafeGuard
계약을 체결함과 동시에 다음 사항을 정의합니다. __abdicate, __queueSetTimelockPendingAdmin 및 __executeSetTimelockPendingAdmin 기능은 있지만 구현되지 않았습니다.
코드베이스의 정확성과 일관성을 향상하려면 ISafeGuard
정확히 일치하는 인터페이스 SafeGuard
구현.
업데이트 : 커밋에서 수정됨 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L04] 독스트링 누락
코드 베이스의 일부 계약 및 기능에는 문서가 부족합니다. 예를 들어, 일부 기능들 FBI 증오 범죄 보고서 SafeGuard
계약.
게다가 일부 독스트링은 다음과 같은 비공식 언어를 사용합니다. 위로 setTimelock
기능의 SafeGuard
계약.
이는 보안뿐만 아니라 정확성도 올바르게 평가하는 데 기본이 되는 코드 의도에 대한 검토자의 이해를 방해합니다. 또한 독스트링은 가독성을 높이고 유지 관리를 용이하게 합니다. 함수의 목적이나 의도, 함수가 실패할 수 있는 시나리오, 함수 호출이 허용된 역할, 반환된 값 및 발생된 이벤트를 명시적으로 설명해야 합니다.
계약 공개 API의 일부인 모든 기능(및 해당 매개변수)을 철저하게 문서화하는 것을 고려하세요. 공개되지 않더라도 민감한 기능을 구현하는 기능도 명확하게 문서화해야 합니다. 독스트링을 작성할 때 다음을 고려하십시오. 이더리움 내추럴 사양 형식 (NatSpec).
업데이트 : 부분적으로 수정 됨 PR # 10. 코드 베이스 전반에 걸쳐 다양한 함수에 적절한 독스트링이 추가되었습니다. 그러나 현재 변경 사항 외에도 다음과 같은 변경을 고려해보세요.
- 추가
description
로@param
위의 독스트링에서queueTransactionWithDescription
기능 - 추가
@param
FBI 증오 범죄 보고서 독스트링 위로createSafeGuard
~에 기능하다SafeGuardFactory
계약 - 추가
@return
함수 위의 독스트링에SafeGuardFactory
계약.
[L05] 쓸모없거나 반복되는 코드
코드베이스에는 코드가 반복되거나 필요하지 않은 위치가 있습니다. 몇 가지 예는 다음과 같습니다:
- 29-32 행 의
Registry
계약은 쓸모가 없기 때문에_add
기능EnumerableSet
계약 이미 이러한 검사를 수행하고 있습니다. 이미 설정된 값에 대해. - 62 행, 67, 73 와 78 의
SafeGuard
계약은 모두 똑같은 작업을 반복합니다. 코드 중복을 방지하려면 내부 함수로 캡슐화하는 것이 좋습니다. - 62-63 행 와 67-68 of
SafeGuard
반복됩니다. 이를 단일 내부 함수로 캡슐화하는 것을 고려하세요. - 사용법
gasleft
함수 호출 시 얼마나 많은 가스를 전달해야 하는지 지정executeTransaction
불필요합니다. 그 실행 시점에서 남은 가스 전체가 실행을 계속하는 데 사용되기 때문입니다. 명시적이지 않은 경우 삭제를 고려하세요.gas
호출의 매개변수입니다.
더 깔끔한 코드를 생성하고 코드베이스에 대한 일관성과 모듈성을 향상하려면 제안된 수정 사항을 적용하는 것이 좋습니다.
업데이트 : 고정됨 PR # 10 그리고 커밋 7fd27df16fc879d990d36a167a0b6e719e578558
.
참고 및 추가 정보
[N01] 일관되지 않은 스타일
코드 베이스에는 스타일 차이가 가독성에 영향을 미쳐 코드를 이해하기 어렵게 만드는 부분이 있습니다. 몇 가지 예는 다음과 같습니다:
- XNUMXD덴탈의
Registry
계약은 전체 계약의 독스트링에 대해 서로 다른 스타일을 사용합니다. - XNUMXD덴탈의
SafeGuard
계약이 이벤트를 내보내는 경우queueTransactionWithDescription
호출되었지만 트랜잭션을 처리하는 다른 함수에서는 이벤트가 발생하지 않습니다. - .
SafeGuard
계약하고 가끔 가치 명명된 매개변수로 사용되며 때로는 _값 사용.
일관된 코딩 스타일이 프로젝트의 가독성에 추가하는 가치를 고려하여 Solhint와 같은 Linter 도구의 도움으로 표준 코딩 스타일을 적용하는 것을 고려하십시오.
업데이트 : 고정됨 PR # 10 그리고 커밋 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N02] 라이센스 누락
코드 베이스 내의 다음 계약에는 SPDX 라이선스 식별자가 없습니다.
컴파일러 경고를 무시하고 코드베이스 전체의 일관성을 높이려면 라이선스 식별자를 추가하는 것이 좋습니다. 그것을하는 동안 참고를 고려하십시오 spdx.dev 가이드 라인.
업데이트 : 고정됨 PR # 10 그리고 커밋 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N03] OpenZeppelin 계약의 종속성이 고정되지 않음
향후 업데이트에서 주요 변경 사항이 릴리스되는 경우 예기치 않은 동작을 방지하기 위해 OpenZeppelin Contracts' 라이브러리, 이 종속성의 버전을 package.json 파일.
업데이트 : 고정됨 PR # 10.
[N04] Solidity 컴파일러 버전이 고정되지 않았습니다.
코드 베이스 전반에 걸쳐 버전 고정을 고려하세요. 솔리디티 컴파일러 최신 안정 버전으로. 이는 호환되지 않는 향후 릴리스로 인해 예상치 못한 버그가 발생하는 것을 방지하는 데 도움이 됩니다. 특정 버전을 선택하려면 개발자는 프로젝트에 필요한 컴파일러 기능과 알려진 버그 목록 각 Solidity 컴파일러 버전과 연관되어 있습니다.
업데이트 : 고정됨 PR # 10.
[N05] 오타
코드 베이스 전체의 다양한 경우에 다음 단어가 사용됩니다. role
다음과 같이 철자가 틀렸습니다. rol
. 그러한 예 중 하나는 다음과 같습니다. 내부의 독스트링 constructor
의 SafeGuard
계약.
코드 가독성을 높이려면 이러한 오타를 수정하는 것이 좋습니다.
업데이트 : 부분적으로 수정 됨 PR # 10. 철자를 쓰는 동안 role
수정되었으므로 "정의된 관리자 주소로 관리자 역할 설정"이라는 설명이 "정의된 관리자 주소로 관리자 역할 설정"으로 변경되어야 합니다. 또한 "execute"의 철자가 잘못되었습니다. SafeGuard
계약하다 라인 69, 라인 82, 라인 96 와 라인 110 그리고 "available"의 철자가 틀렸습니다. 라인 70, 라인 83, 라인 97, 라인 111. 또한 다음과 같은 비공식적인 단어를 바꾸는 것도 고려해 보세요. “할 거야” in SafeGuard
“going to”와 같은 형식적인 대안을 사용한 계약.
[N06] uint를 uint256으로 선언합니다.
코드베이스에는 변수가 선언되는 경우가 여러 번 있습니다. uint
대신 데이터 유형 uint256
. 예를 들어, eta
의 변수 QueueTransactionWithDescription
event 의 SafeGuard
계약.
명확성을 높이기 위해 모든 인스턴스는 uint
다음과 같이 선언되어야 한다. uint256
.
업데이트 : 고정됨 PR # 10 그리고 커밋 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N07] 미사용 수입품
XNUMXD덴탈의 SafeGuard
계약은 console
계약했지만 절대 사용하지 마세요.
코드의 가독성을 높이려면 사용하지 않는 가져오기를 제거하는 것이 좋습니다.
업데이트 : 고정됨 PR # 10.
결론
한 가지 높은 취약점과 기타 여러 가지 사소한 취약점이 발견되었으며 권장 사항과 수정 사항이 제안되었습니다.
- &
- ACCESS
- 계정
- 가로질러
- 동작
- 행위
- 추가
- 주소
- 관리자
- All
- 이미
- 이기는하지만
- API를
- 접근
- 약
- 회계 감사
- 원래
- 존재
- 버그
- 사업
- 전화
- 가지 경우
- 원인
- 이전 단계로 돌아가기
- 요금
- 암호
- 코딩
- 공통의
- 화합물
- 혼동
- 고려
- 이 포함되어 있습니다
- 계속
- 계약
- 계약
- 수
- 창조자
- Current
- 데이터
- 취급
- 디자인
- 개발자
- 다른
- 발견
- 정교한
- ETH
- 이벤트
- 이벤트
- 예
- 공장
- 특징
- 최종적으로
- 먼저,
- 수정
- 유연성
- 발견
- 기능
- 자금
- 미래
- 가스
- 기부
- 골
- 통치
- 지사
- 가이드 라인
- 데
- 도움
- 여기에서 지금 확인해 보세요.
- 높은
- 방법
- HTTPS
- 구현
- 증가
- 증가
- 인터페이스
- IT
- 알려진
- 언어
- 최근
- 도서관
- 특허
- 라인
- 명부
- 지방의
- 고정
- 보고
- 유튜브 영상을 만드는 것은
- 경기
- 모듈러
- Multisig
- 기타
- 제시
- 방법
- 프로젝트
- 신청
- 공개
- 게시
- 회원가입
- 보도 자료
- 신고
- 저장소
- 결과
- 리뷰
- 보안
- 세트
- 설정
- 공유
- 스마트 한
- 스마트 계약
- solidity
- 주 정부
- 스타일
- 체계
- 을 통하여
- 도처에
- 시간
- 검색을
- 선로
- 거래 내역
- 업데이트
- us
- 사용자
- 가치
- 취약점
- 지갑
- 주
- 누구
- 이내
- 말
- 쓰기