Hubii 토큰 감사

Hubii 팀은 크라우드 세일 및“Hubiits”(HUB) 토큰 계약을 검토하고 감사하도록 요청했습니다. 코드를 살펴보고 결과를 게시했습니다.

감사 된 계약은 CoinFabrik / ico 저장소에서 찾을 수 있습니다. 이 보고서에 사용 된 버전은 commit 142f43f3bec4afd0ffff3fce06946cf67cb0272f 입니다.

코드가 매우 모듈화되고 잘 문서화되어 있음을 발견했습니다. 다음은 중요도 순으로 평가 및 권장 사항입니다.

업데이트 : Hubii 팀은 최신 버전의 코드에서 대부분의 권장 사항을 따랐습니다.

심각

상태 전환은 암시 적이며 시행되지 않습니다.

Crowdsale 인스턴스가있을 수있는 여러 상태가 있습니다. 현재 상태는 getState 를 호출하여 얻을 수 있으며 이에 따라 기능이 활성화 또는 비활성화됩니다. 이 기능은 크라우드 세일이 따라야 할 원하는 상태 시퀀스에 대한 아이디어도 제공합니다. 그러나 암시 된 것보다 훨씬 많은 가능한 상태 전환이 있다는 것은 쉽게 알 수 있습니다.

이러한 종류의 예상치 못한 상태 전환은 예상치 못한 결과를 초래하고 보안 허점을 만들고 유지 관리 및 추가 개발을 복잡하게 만들 수 있습니다. 전환을 명시 적으로 만들고 각 함수를 호출 할 수있는 상태를 적용하여이 문제를 해결하세요.

올바르게 구현 된 명시 적 상태 전환의 몇 가지 예는 OpenZeppelin의 RefundVault 및 Solidity의 문서의 StateMachine입니다.

업데이트 : Hubii는 “상태 전환이 가능한 기능은 onlyOwner , 외부 공격 가능성을 낮 춥니 다. onlyOwner 한정자가있는 함수에도 불구하고 예기치 않은 상태 전환을 방지하기 위해 계약을 수정하고 준비 상태.” 5a90957abf3f792951b3aad66d6eb0317f0500ca 에서 수정되었습니다.

잘못된 투자자 수

Crowdsale.sol에서 사전 할당 호출은 investorCount 변수를 증가시키지 않습니다. 이 기능을 사용하여 자금을 이체하는 투자자를 계산하려는 경우이 방법에서 변수를 늘리는 것이 좋습니다.

업데이트 : Hubii는이 변수의 의미는 의도적으로 설계된 것이라고 답했습니다. Hubii는 investorCount 는 사전 할당 기능을 통해 할당 된 초기 투자자가 아닌 크라우드 세일에서 투자자를 계산하도록 설계되었습니다. 이런 방식으로 설계되었습니다.”

또한 investedAmountOf 에 대한 191 행 체크인은 계산 된 가격이 0 인 경우 일반 구매에 대한 투자자 수를 증가시키지 않습니다. 대신 tokenAmountOf 를 확인하도록 가드를 변경하는 것이 좋습니다.

업데이트 : Hubii는“0이 아닌 금액을 보내려면 정기적으로 구매해야하며 investorCount investedAmountOf 가 0 일 때만 증가하여 동일한 주소가 두 번 계산되는 것을 방지합니다. 크라우드 세일을 통해 토큰을 구매 한 초기 투자자도 investedAmountOf 확인을 통해 올바르게 계산되며, 확인하면 발생하지 않습니다. tokenAmountOf .”

계산 가격 인수 순서가 일치하지 않음

calculatePrice 함수는 PricingStrategy 에 정의되어 weiRaised tokensSold 를 포함한 5 개의 인수를받습니다. 이 두 가지는 FlatPricing 의 함수 구현과 Crowdsale 에서의 사용에서 바뀝니다. 구현 및 사용이 일관 적이기 때문에 현재 문제가되지 않습니다. 문서화 된 주문을 사용하여 새로운 가격 책정 전략을 구현하면 계약이 깨질 수 있습니다. 동일한 순서를 사용하도록 인터페이스를 수정합니다 : calculatePrice (uint value, uint weiRaised, uint tokensSold, address msgSender, uint decimals) .

업데이트 : Hubii 답변 “인수의 이름과 문서는 실제 사용법을 반영하도록 수정되었습니다. 하지만이 문제는 계약 행위에 영향을주지 않기 때문에 심각하지 않습니다.” c3fc1798313e3c2bd98decb57f97f74e5f003209 에서 수정되었습니다.

토큰 계약 생성자는 토큰 방출 및 해제를 제어합니다.

대부분의 구현 에서처럼 (OpenZeppelin의 Crowdsale 참조) 토큰의 소유자 역할을하는 Crowdsale 대신 토큰의 소유권을 보유하는 계정이 있습니다. 이를 통해 소유자는 크라우드 세일 중 언제든지 자신을 채굴 대리인으로 지정하고 새로운 토큰을 발행하거나 크라우드 세일이 끝나기 전에 토큰을 릴리스 할 수 있습니다. 이것은 크라우드 세일 메커니즘에 대한 대중의 신뢰를 떨어 뜨립니다. 토큰의 소유권을 Crowdsale 계약으로 이동하고 팀을 upgradeMaster 로 지정하여 업그레이드 메커니즘에 대한 제어 만 유지합니다.

업데이트 : Hubii는 “다른 문제에서 제안한대로 단일 계약에서 모든 계약을 생성하도록 설정이 변경되었습니다. 이러한 방식으로 Crowdsale 계약은 CrowdsaleToken .” e9e05d7adc9efde724afee0facfc89e76db67efe 에서 수정되었습니다.

전략 및 제한은 크라우드 세일 중 수정 가능

Crowdsale의

메소드 setEndsAt , setPricingStrategy , setCeilingStrategy , setFinalizeAgent setFundingCap .sol을 사용하면 크라우드 세일이 실행되는 동안 소유자가 크라우드 세일 규칙을 변경할 수 있으며, 크라우드 세일이 종료되는 시점, 펀드 한도, 가격 및 주소 당 최대 투자가 어느 단계에서든 변경됩니다. 이를 위해서는 크라우드 세일 소유자에 대한 완전한 신뢰가 필요합니다. 건설시 이러한 매개 변수를 요구하거나 크라우드 세일의 준비 단계로만 이러한 변경을 제한하는 것을 고려하십시오.

업데이트 : Hubii는 “이러한 메서드는 Crowdsale 계약이므로 나중에 하도급을 변경할 수 없습니다.” 5a90957abf3f792951b3aad66d6eb0317f0500ca 에서 수정되었습니다.

사전 할당 기능에는 온 전성 검사가 없습니다.

Crowdsale.sol L239의

preallocate 는 토큰의 양이 0이 아닌지, 수신자 주소가 0이 아닌지, Crowdsale 의 상태가 PreFunding ( Failed 또는 Finalized 크라우드 세일에서도 사전 할당이 전송되도록 허용) 또는 한도를 초과하지 않습니다. 메소드가 Crowdsale 소유자에 의해서만 호출되는 경우에도 인적 오류로 인한 불일치 상태를 방지하기 위해 이러한 모든 조건에 대한 검사를 추가하는 것이 좋습니다.

업데이트 : Hubii는 사전 할당 기능을 사용하여 문제를 피할 수 있습니다.” b4fccc6dee752f9b64b5598270032fd9cec106f6 에서 수정되었습니다.

잠재적 인 문제

계약은 불필요하게 일반적이고 복잡합니다.

Crowdsale CrowdsaleToken 계약 모두 선택적 동작을 제어하기 위해 여러 플래그에 의존합니다. 몇 가지 예는 다음과 같습니다.

다양한 플래그와 사용 가능한 동작이 여러 방식으로 상호 작용하므로 매우 복잡한 계약이 생성됩니다. 복잡성이 클수록 공격 표면이 커집니다. 플래그를 통해 조건부로 다른 동작을 제어하는 ​​대신 클래스 당 단일 동작을 설정하여 복잡성을 줄이는 것이 좋습니다.

업데이트 : Hubii는 “계약에서 여러 옵션을 제공해야하는 경우 약간의 복잡성이 필요합니다. 현재 개발 단계에서 계약을 크게 변경하는 것은 너무 위험하다고 생각합니다.”

계약 초기화가 너무 복잡합니다.

크라우드 세일 계약에 전략 패턴을 과도하게 사용하면 배포 및 구성이 훨씬 더 복잡해지고 여러 에이전트 구성 ( setMintAgent , setReleaseAgent , setFinalizeAgent ), 오류가 발생하기 쉽습니다. 에이전트 및 토큰 계약 설정 및 구성을 포함하여 필요한 모든 초기화 단계를 수행하는 Crowdsale 의 특수 버전 ( HubiiCrowdsale 이라고도 함)을 정의하는 것이 좋습니다.

참고로 모든 setAgent 메소드는 생성 중뿐만 아니라 언제든지 호출 할 수 있습니다. 이것은 패리티 MultiSig 해킹을 가능하게하는 문제 중 하나 였지만이 경우에는 onlyOwner 수정자가 유사한 공격을 방지합니다.

업데이트 : Hubii는 “작년 메인 넷의 가스 한도를 우회하기 위해 계약을 여러 부분으로 분할했습니다. 지금은 이것이 문제가되지 않는다는 것을 알고 있으며 단일 계약 내에서 설정 프로세스를 그룹화했습니다. " 5a90957abf3f792951b3aad66d6eb0317f0500ca 에서 수정되었습니다.

소유자는 필요한 것보다 더 많은 환불 금액을로드 할 수 있습니다.

loadRefund 를 사용하면 잔액을 계약에 보내 환불에 사용할 수 있습니다. 이 금액은 refund 기능을 통해서만 인출 할 수 있으며, 투자자는 투자 한 금액과 동일한 금액의 환불을 요청해야합니다.

필요한 것보다 더 많은 잔액을로드 할 수 있으며 계약은 추가 잔액을 인출 할 수있는 메커니즘을 제공하지 않습니다. (아래에 설명 된대로 사전 할당 의 안전 검사 부족을 악용하여이를 철회 할 수 있지만, 이에 의존해서는 안됩니다.) msg.value = = loadRefund 의 weiRaised .

업데이트 : Hubii는 “환불 이더의 초과분을 보낸 사람 주소에 할당하기로 결정했습니다. 따라서 그들은 정상적인 메커니즘을 통해 초과분을 환불 할 수 있습니다.” b6f8f29e8e6578d9efc56457c0aa8ff3a49f2c05 에서 수정되었습니다.

크라우드 세일에 파이널 라이즈 에이전트가 필요한지 여부 불분명

종료 기능의 코드에 “종료 화는 선택 사항입니다. 마무리 에이전트가 지정된 경우에만 호출합니다.”, Crowdsale 은 종료 에이전트가 설정되지 않은 경우 L481에 따라 준비 중 상태에서 벗어날 수 없습니다. .

마무리 에이전트를 필수로 지정하려는 경우 생성자 매개 변수의 일부로 수락하고 즉시 변경하는 메서드를 제거합니다. 반면, 최종화 에이전트를 지원하지 않으려는 경우 토큰을 해제 할 수있는 대체 방법이 있는지 확인하십시오. 이는 현재 최종화 에이전트의 책임입니다.

업데이트 : Hubii는 “다른 문제의 일환으로 준비 중 단계가 제거되었습니다. 이제 finalizeAgent 는 필수입니다.”

자금 수령 주소는 소유자가 잠시 동안 변경할 수 있습니다.

setMultisig 함수를 사용하면 소유자가 자금이 송금되는 주소를 변경할 수 있습니다. 크라우드 세일이 전개 된 후 투자자 수가 5 개가되기 전까지 잠시 사용할 수 있습니다. 댓글에 명시된 목적은 크라우드 세일이 시작되기 전에 수정할 수 있도록하는 것이지만, 이는 계약에 의해 강제되지 않습니다.

투자자가 5 명 미만인 크라우드 세일이 이미 진행중인 경우 소유자는 여전히 주소를 변경할 수 있습니다. 실수 나 악의로 잘못 설정하면 곧 투자자 수가 5 개가되면 변경이 불가능 해집니다. 그렇기 때문에 현재로서는 좋은 안전 조치가 아닙니다. 함수를 제거하거나 onlyInEmergency 수정자를 추가하는 것이 좋습니다.

업데이트 : Hubii는 “수신 주소 변경 기능이 내부 에 선언되었습니다. 계약이 배포 될 때만 수정을 허용합니다.” 5a90957abf3f792951b3aad66d6eb0317f0500ca 에서 수정되었습니다.

Ownable의 transferOwnership이 조용히 실패

transferOwnership 에는 주소 인수가 0이 아닌지 확인하기위한 온 전성 검사가 있으며 이는 자동으로 실패 할 수 있습니다. 검사가 통과되지 않으면 줄을 require (newOwner! = 0x0); 로 변경하여 던지는 것을 고려합니다.

업데이트 : Hubii는 transferOwnership 기능이 newOwner to be non null.” 5a90957abf3f792951b3aad66d6eb0317f0500ca 에서 수정되었습니다.

모든 수학 연산에서 SafeMath 사용

대부분의 수학 연산은 안전하지만 확인되지 않은 몇 가지 연산이 있습니다 (예 :이 두 연산). 항상 안전하고 점검 작업을 수행하는 것이 좋습니다.

업데이트 : Hubii는 “사용되지 않는 안전한 수학 연산을 사용하도록 업데이트했습니다.”라고 답했습니다.

전략 및 에이전트 계약을 유효하지 않은 계약으로 설정할 수 있습니다.

전략 계약 PricingStrategy CeilingStrategy FinalizationAgent 는 각각 isSane 함수와 함께 Crowdsale . 명백한 목적은 Crowdsale 인스턴스와 관련하여 올바른 설정을 확인하는 것입니다. 인스턴스는 크라우드 세일 중에 변경할 수 있지만 유효성이 검증되지 않고 계약이 유효하지 않은 상태로 남을 수 있습니다. setter setPricingStrategy , setCeilingStrategy setFinalizeAgent 에서 각각에 대한 유효성 검사를 추가하는 것이 좋습니다. 예를 들어 396 행 뒤에 isPricingSane (); 을 추가합니다.

업데이트 : Hubii는 “이제 크라우드 세일 중 수정을 방지하기 위해 setter가 내부에 있으며 유효한지 확인합니다.”라고 답했습니다. 5a90957abf3f792951b3aad66d6eb0317f0500ca 에서 수정되었습니다.

경고

솔리 디티 버전

현재 코드는 버전 pragma ^ 0.4.11 을 지정합니다. Solidity 버전 pragma를 최신 버전 ( pragma solidity ^ 0.4.14 )으로 변경하여 최신 컴파일러를 사용하는 것이 좋습니다. 특히 최신 버전을 고려하면 Crowdsale.sol이 사용하는 ecrecover 기능에 대한 보안 수정 사항이 포함되어 있습니다.

업데이트 : Hubii는 “솔리 디티 v0.4.13으로 요구 사항을 업데이트하고 ecrecover 사용을 제거했습니다. code> Crowdsale 에 필수적이지 않았기 때문입니다. 아직 Truffle에서 지원되지 않기 때문에 v0.4.14로 업데이트하지 않았습니다.”

컴파일러 경고

컴파일러는 변수가 저장 포인터로 선언되었다는 경고를 발행합니다. MultiSigWallet.sol에서이 경고를 끄려면 명시적인 "저장"키워드를 사용하십시오. 이 경고를 제거하려면 필수 키워드를 추가하십시오.

업데이트 : Hubii는 '추천 한대로 키워드 저장소를 추가했습니다.'라고 답했습니다. 5a90957abf3f792951b3aad66d6eb0317f0500ca 에서 수정되었습니다.

ERC20 소수 필드는 uint8이어야합니다.

토큰의 소수 필드는 CrowdsaleToken FractionalERC20 모두에서 uint256 으로 정의됩니다. 그러나 ERC20 표준에서는 uint8 로 정의되어 있습니다. 유형을 uint8 로 변경해보세요.

업데이트 : Hubii는 “표준 및 선언 십진수를 반영하도록 계약을 업데이트했습니다. uint8 .” 312d8be8002dc99074dbd2a0a6db1075cc5623ed 에서 수정되었습니다.

UpgradeAgent의 문서화되지 않은 인터페이스

나머지 코드에 비해 UpgradeAgent 는 문서와 명확성이 부족합니다. 이것은 미래의 개발자가 사용하기위한 것이므로 매우 잘 문서화하는 것이 특히 중요하다고 생각합니다.

이 함수는 이전 토큰에 의해 호출됩니다.

매개 변수의 이름을 소유자 및 양으로 바꾸는 것을 고려하십시오. 이는 그 의미를 더 잘 나타냅니다. 또한 upgradeFrom UpgradeAgent 에 적용하는 것을 고려하십시오.이전 토큰에서만 호출 할 수 있습니다.

업데이트 : Hubii는 UpgradeAgent 에 문서를 추가했습니다. 기능.” 고정 a042810f569192b074f22f33df3b52c6cdafe53a 에서.

낮은 테스트 범위

코드베이스에는 Crowdsale 계약에서 몇 가지 메서드 만 확인하는 단일 테스트 파일이 있습니다. 마무리, 환불,크라우드 세일 실패, 사전 할당, 고객 ID 및 서명을 사용한 투자 등 릴리스 및 업그레이드 메커니즘에 대한 테스트를 포함하여 CrowdsaleToken 과 같은 다른 계약에 대한 테스트도 포함됩니다.

또한모든 테스트는 동일한 배포 된 인스턴스 집합에서 실행됩니다. 이는 모든 테스트가 단일 구성에서 연속적으로 실행되도록 제한하여 모든 계약에 대한 단일 생성자 매개 변수 집합으로 테스트를 제한합니다. 서로 다른 테스트 스위트에서 서로 다른 인스턴스를 설정하여다양한 구성을 수용하고 필요한 경우 모의를 사용하여 모든 계약에 대한 단위 테스트를 추가합니다.

업데이트 : Hubii는 “감사 이후 여러 테스트를 추가했으며 더 추가하고 있습니다.”라고 답했습니다.

패키지 관리자를 통해 라이브러리를 설치하지 않음

코드는 OpenZeppelin의 BasicToken , ERC20 , ERC20Basic , Ownable SafeMath <를 사용하지만 / code>,코드는 uint256 의 이름을 uint 로 변경하는 것과 같은 일부 스타일 수정을 포함하여 저장소에 복사 및 붙여 넣기됩니다. 따라서 수동으로 통합해야하므로 라이브러리의 새 릴리스에 포함 된 보안 변경 사항을 포함하기가 어렵습니다.OpenZeppelin 설치에 npm 사용을 고려하십시오.

업데이트 : Hubii는 “저장소의 향후 변경을 위해이를 고려할 것입니다.”라고 답했습니다.

Crowdsale 용 토큰은 Mintable이어야합니다.

invest 메소드에서 토큰 계약은 MintableToken 으로 강제됩니다. 토큰 필드 유형을 MintableToken 으로 변경해보세요.

또한 CrowdsaleToken 계약이 Crowdsale 의 토큰으로 사용되며, 크라우드 세일이 작동하려면 생성자에서 토큰의 플래그 mintable 을 true로 설정해야합니다. 이 플래그를 모두 제거하고 항상 mintable == true 를 만드는 것이 좋습니다.

CrowdsaleToken Crowdsale <의이 인스턴스에서 생성 가능합니다. em>.”

환불은 신뢰할 수 없습니다

크라우드 세일은최소 한도에 도달하지 않은 경우 원래 투자자에게 자금을 반환하는 논리 (L437 참조). 그러나 이는 loadRefund 메소드를 통해 자금을 수동으로 Crowdsale 로 다시 이체하는 팀에 따라 달라지며대중이 환급을 효과적으로 발행하기 위해 제 3자를 신뢰합니다. 크라우드 세일이 실패하거나 성공한 것으로 알려질 때까지 특별히 설계된 계약에 자금을 잠그고 후자의 경우에만 자금을 팀으로 이체하는 것을 고려하십시오.

하지만 향후 참조를 위해 고려할 것입니다.”

타임 스탬프 사용

계약 로직에 타임 스탬프 및 now ( block.timestamp 의 별칭)를 사용하는 데 문제가 있습니다.채굴자가 조작을 수행 할 수 있다는 사실을 기반으로합니다. 일반적으로 계약 로직의 타임 스탬프에 의존하지 않는 것이 좋습니다. 해결 방법은 대신 block.number를 사용하고 예상 블록 높이와 예상 블록 양의 기간으로 대략적인 날짜를 사용하는 것입니다.

이 조작의 잠재적 인 위험이 있으며 필요한 경우 block.number 로 전환하십시오.

이 주제에 대한 자세한 내용은이 스택 교환 질문을 참조하세요.

업데이트 : Hubii는 “타임 스탬프 대신 블록 번호를 사용하도록 계약이 업데이트되었습니다.” 9db3e7cbdd4d33f7c2d2135f9f536d7e5bc73144 에서 수정되었습니다.

참고 및 추가 정보

결론

우리는코드는 매우 모듈화되고 잘 문서화됩니다. 6 개의 심각한 보안 문제와 해결 방법이 발견 및보고되었습니다. 모범 사례를 따르고 잠재적 인 공격 표면을 줄이기 위해 몇 가지 변경 사항이 제안되었습니다.

업데이트 : Hubii 팀이 명확히 함발견 된 문제 중 일부는 취약점이 아닌 설계 결정이었습니다. 팀은 권장 사항에 따라 대부분의 문제를 해결했습니다.

게시일 현재 위 리뷰는 현재 이해하고있는 내용입니다.HUB 토큰 및 크라우드 세일 계약과 관련하여 알려진 보안 패턴입니다. 관련 Hubii 프로젝트를 검토하지 않았습니다. 위 내용은 투자 조언이나 HUB 토큰 제공으로 해석되어서는 안됩니다. 스마트 계약 보안에 대한 일반적인 정보는 여기 에서 우리의 생각을 확인하세요.

커뮤니티에 참여