smart contract security를 improve해주는 전략과 이 전략을 따르지 않았을 때 문제가 발생할 수 있는 코드를 함께 제시한다. 또한 smart contract를 protect할 수 있는 방법에 대한 코드도 함께 제시한다.
- code가 silently하게 fail하는 것을 피해야 한다. 또한 unstable하거나 inconsistent한 state로 실행이 계속되는 것을 피해야 한다.
//Bad code
// UNSAFE CODE, DO NOT USE!
contract BadFailEarly {
uint constant DEFAULT_SALARY = 50000;
mapping(string => uint) nameToSalary;
function getSalary(string name) constant returns (uint) {
if (bytes(name).length != 0 && nameToSalary[name] != 0) {
return nameToSalary[name];
} else {
return DEFAULT_SALARY;
}
}
}getSalary 함수 내에서 stored salary를 리턴하기 전에 condition을 checking하는 것은 good thing이다. 그러나 문제는, 이러한 condition이 충족되지 않았을 때 DEFAULT_SALARY가 리턴된다. 이는 caller에게 error를 숨기는 행위다. the sooner the fail, the easier it will be to find the problem. 이러한 행위로 인해 error는 숨겨지게 되고 다른 code 부분에 전파되어 결국 inconsistencies를 일으킨다.
//Good code
contract GoodFailEarly {
mapping(string => uint) nameToSalary;
function getSalary(string name) constant returns (uint) {
if (bytes(name).length == 0) throw;
if (nameToSalary[name] == 0) throw;
return nameToSalary[name];
}
}위의 두 개의 precondition을 각각으로 분리시켜 어느 부분에서 error가 검출되는지 알아야 한다. 이러한 checking은 modifier를 이용해서도 이루어진다.
이 말의 뜻은 자동으로 돈이 push되게 하지 말고 사용자가 직접 pull하게 한다. 모든 Ether transfer는 code 실행을 잠재적으로 가진다. contract는 Ether를 받게 되면 fallback 함수를 호출하게 되는데 이 fallback 함수에 error가 있을 수도 있다. 따라서 Ether를 send하는 함수가 error를 발생시키지 않을 것이라고 확신하면 안된다.
//Bad code
contract BadPushPayments {
address highestBidder;
uint highestBid;
function bid() {
if (msg.value < highestBid) throw;
if (highestBidder != 0) {
// return bid to previous winner
if (!highestBidder.send(highestBid)) {
throw;
}
}
highestBidder = msg.sender;
highestBid = msg.value;
}
}우선은 이 코드에서 bid 함수안에 send 함수를 호출하고 이 함수에 대한 결과값(true/false)을 check한 후 highestBidder와 highestBid의 값을 setting한다. 얼핏보면 문제가 없어보이지만 명백하게 unsafe하다. 만약 highestBidder가 contract이고 이 contract의 fallback 함수가 error를 발생시키면 어느 누가 이 bid 함수를 호출하던 상관없이 항상 error를 리턴한다. 왜냐하면 bid 함수가 호출될 때마다 fallback 함수에서 error를 발생시키기 때문이다.
그렇기 때문에 highestBidder에게 돈을 돌려주는 로직을 자동으로 어떤 로직이 발생했을 때 push되는 방식이 아니라, user가 직접 pull하는 방식을 취해야 한다. 즉, user가 돈을 빼내는 로직의 함수를 따로 작성해야 한다.
//Good Code
contract GoodPullPayments {
address highestBidder;
uint highestBid;
mapping(address => uint) refunds;
function bid() external {
if (msg.value < highestBid) throw;
if (highestBidder != 0) {
refunds[highestBidder] += highestBid;
}
highestBidder = msg.sender;
highestBid = msg.value;
}
function withdrawBid() external {
uint refund = refunds[msg.sender];
refunds[msg.sender] = 0;
if (!msg.sender.send(refund)) {
refunds[msg.sender] = refund;
}
}
}새로 추가된 코드는 mapping(address => uint) refunds 상태 변수와 withdrawBid() 함수다. 이렇게 함으로써 send 함수를 호출하는 주체는 contract에 bid를 호출하는 user가 아니라, 본인만이 이 send 함수에 책임을 지게 된다. when sending ether, favor pull over push payments
fail-early principle의 확장으로서 function을 만들 때 좋은 practice가 있다:
- 먼저 모든 pre-conditions을 check한다.
- 그 다음 contract's state를 변경한다. (바꿀게 없으면 생략한다.)
- 마지막으로 other contract와 interact해야 한다면, 이와 관련된 코드는 가장 나중에 처리한다.
이렇게 Conditions, actions, interactions의 구성은 contract가 가질 수 있는 많은 문제점을 줄일 수 있다.
function auctionEnd() {
// 1. Conditions
if (now <= auctionStart + biddingTime)
throw; // auction did not yet end
if (ended)
throw; // this function has already been called
// 2. Effects
ended = true;
AuctionEnded(highestBidder, highestBid);
// 3. Interaction
if (!beneficiary.send(highestBid))
throw;
}
}pre-conditions을 위에 두는 이유는 fail-early의 원칙과 비슷한 맥락이고, 마지막으로 onter contract와 interaction하는 부분은 potentially dangerous하기 때문에 가장 끝에 둔다.
EVM은 contract가 할 수 있는 것들에 대한 엄격한 제한이 있다. 이러한 제한은 platform-level security considerations이다. 하지만 이러한 제한을 모르면 contract's security에 문제를 발생시킬 수도 있다.
//Unsafe code
contract BadArrayUse {
address[] employees;
function payBonus() {
for (var i = 0; i < employees.length; i++) {
address employee = employees[i];
uint bonus = calculateBonus(employee);
employee.send(bonus);
}
}
function calculateBonus(address employee) returns (uint) {
// some expensive computation ...
}
}이 코드는 매우 직관적이고 문제가 없어 보인다. 하지만 platform limits의 관점으로 3가지 문제점을 가지고 있다.
첫 번째 문제는 i의 type이 var다. 즉, uint8이다. 만약 i의 값이 255를 넘어가게 되면 0이 되고 이는 무한 루프에 빠지게 될 가능성이 매우 크다. 결국 이는 gas depletion을 발생시키게 된다. 따라서 var 타입을 uint로 변경한다. 하지만 uint도 uint256과 같이 뒤에 uint의 byte 크기를 명시하는 숫자값을 같이 써주는것이 더욱 좋다.
//Still unsafe code
contract BadArrayUse {
address[] employees;
function payBonus() {
for (uint i = 0; i < employees.length; i++) {
address employee = employees[i];
uint bonus = calculateBonus(employee);
employee.send(bonus);
}
}
function calculateBonus(address employee) returns (uint) {
// some expensive computation ...
}
}두 번째로 고려해야 하는 것은 gas limit이다. 위의 코드를 보면 각각의 employee에 대해, 즉 모든 employee에 대해 calcurateBonus 함수가 작동한다. calcurateBonus 함수가 가스를 많이 소모하는 함수라고 한다면 이러한 동작은 a lot of gas를 소모시키고 금방 gas limit에 도달한다. Be aware of variable gas costs when using loops.
//Still unsafe code
contract BadArrayUse {
address[] employees;
mapping(address => uint) bonuses;
function payBonus() {
for (uint i = 0; i < employees.length; i++) {
address employee = employees[i];
uint bonus = bonuses[employee];
employee.send(bonus);
}
}
function calculateBonus(address employee) returns (uint) {
uint bonus = 0;
// some expensive computation modifying the bonus...
bonuses[employee] = bonus;
}
}마지막으로 call stack depth limit이 있다. EVM에서는 호출되는 함수가 larger than 1024이면 error(stack limit reached 1024)를 발생시킨다. attacker는 1023번 재귀 호출하는 함수를 만들고 contract function을 호출시킬 수도 있다. 이를 해결하기 위한 코드가 OpenZeppelin에서 제공하고 EIP150에서 이러한 문제를 fix했다.
import './PullPayment.sol';
contract GoodArrayUse is PullPayment {
address[] employees;
mapping(address => uint) bonuses;
function payBonus() {
for (uint i = 0; i < employees.length; i++) {
address employee = employees[i];
uint bonus = bonuses[employee];
asyncSend(employee, bonus);
}
}
function calculateBonus(address employee) returns (uint) {
uint bonus = 0;
// some expensive computation...
bonuses[employee] = bonus;
}
}정리하면 (1) limits in the type you're using, (2) limits in the gas costs of your contract, (3) the call stack depth limit 이 3가지 platform limits을 기억해야 한다.
테스팅은 오래 걸리지만 regression problem를 예방할 수 있다. Truffle's testing guide를 참고하길 바란다.
포상금을 제공해라. 에러는 빨리 찾아내면 찾아낼수록 좋다. 자세히 읽지 않음. 포상금을 제공하는 contract를 작성함.
contract를 공격하는 attacker는 돈이 많이 묶여있는 contract를 공격할 가능성이 높다. LIMIT보다 높은 금액을 받을 필요가 없는 contract는 LIMIT을 두어 deposit을 막는다.
contract LimitFunds {
uint LIMIT = 5000;
function() { throw; }
function deposit() {
if (this.balance > LIMIT) throw;
...
}
}contract로의 직접적인 payments는 모두 fallback 함수에서 거절한다.
security는 우리의 의도대로 코드가 동작해야 이루어진다. 만약 코드가 huge하고 messy하다면 우리의 의도대로 코드가 동작하기 힘들다. 그렇기 때문에 contract를 짤 땐 더욱 최대한 simple하고 modular한 code를 짜야 한다. 즉, function들은 최대한 short해야 하고 code dependencies도 최소화해야 한다. 또한 각각의 독립적인 logic은 모듈로 포함되어야 하고, 각각의 함수는 단 하나의 responsibility를 가져야 한다.
변수나 함수의 Naming은 우리의 의도를 반영하는 것이기 때문에 Naming도 매우 중요하다. Naming에 대해 깊게 고민해야 하며 Naming은 결국 가능한 한 clear해야 한다.
함수의 이름을 보고 1분 안에 이 함수가 어떤 역할을 할 것이라고 예상이 되어야 하고, 실제로도 그래야 한다.
contract를 simple하고 modular하고 well-named하게 해야만 code를 auditing할 때 유용하다.
contract는 직접적으로 돈을 건드린다. 또한 contract의 code와 data는 공개되고, 새롭고 실험중인 플랫폼 위에서 작동된다. 그렇기 때문에 모든 코드를 처음부터 작성하긴 힘들고 그렇게 해서도 안된다. 결국 이미 작성된 안전한 코드와 프레임워크를 사용해야 한다.