LoginSignup
22
9

More than 1 year has passed since last update.

【黒歴史を】スマコンエンジニアよ悪意をいだけ【重ねないために】

Last updated at Posted at 2022-08-14

はじめに

みなさん聞き飽きているとは思いますが、何度でも言わせてください。
「EVM系のスマコンはデプロイしたら終わりです」

本当におしまいです。

「だってスマコンは修正がきかないから」

スマコンにバグがあろうものなら、不具合もろともブロックチェーン上に記録され、未来永劫 黒歴史を晒し続けることになります。

諦めてください。
お手上げです。

なのでスマコンのデプロイに際して、事前確認はすごく重要です。
しっかりとした人員と時間をかけ、全ての状況で問題がないことをテスト環境で検証するのが理想です。とはいえ、プロジェクトの規模によってはスマコン担当者以外に、誰もスマコンのチェックを手伝ってくれる人間がいない、なんてことはザラです。

「頼れるのは自分だけ」

そんな、字面だけはかっこいい悲しい状況で、いかに不具合を未然に防ぐか。
デプロイ前のスマコンへの取り組みに関して、私なりの軽いメモを書いてみます。

悪意をもってコードを読もう

スマコンはその役割上、資産(仮想通貨、NFT、FT)の管理とは切っても切り離せません。

で、不具合として一番きついのが「資産が奪われること」。
次が「資産の内容が改ざん/破壊されること」です。

これらの最悪な不具合を残してしまわないよう、スマコンのコードを見直す際は、常に最上級の悪意をもって取り組むようにしましょう。具体的には、スマコンを見たらまず、「金目のデータ」に目星をつけ、次に、「内部データへの書き込み箇所」を把握します。

イメージ的には 「なんとかして資産を引っこ抜いてやろう」「隙あらば中身を滅茶苦茶にしてやろう」 という意識で、ねっとり舐めるようにコードを読み進めるのが理想です。

この時、ポイントとしては下記の部分を特に注意しましょう。
設定ミス(値の桁や日付の指定の間違え)
設定関数の記述ミス(変数の取り違え)
requireのミス(指定漏れ、誤判定)
アクセス制限の指定漏れ

「こんなミス誰もしねぇよw」 と思うかも知れませんが、個人的には、これらの要因による不具合が圧倒的に多いと思います(特に、開発終盤、プロジェクト都合で 「かんたんな機能」 を追加するような時ほど、おそろしい魔物がするりと紛れ込む印象があります)。

世にもダメなスマコン: BuggyToken

ダメなスマコンの例を見てみましょう。
下記は、いわゆるNFTのスマコンで、NFTの販売(mint)機能を提供します。デプロイ後、問題なくmintができますし、OpenSeaにコレクションも作成されます。

ですが、先にあげた不具合の要因をもれなく含んでいるため、本番環境へデプロイしようものなら、黒歴史になること請け合いです。
どこが問題かわかりますか?

// SPDX-License-Identifier: MIT
pragma solidity >=0.8.7 <0.9.0;

import "@openzeppelin/contracts/utils/Strings.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/token/ERC721/ERC721.sol";

//------------------------------------------------------------
// バグったトークン(このコントラクトをコピペして使わないこと!)
//------------------------------------------------------------
contract BuggyToken is Ownable, ERC721 {
    //--------------------------------------------------------
    // 定数
    //--------------------------------------------------------
    string constant private TOKEN_NAME = "Buggy Token";
    string constant private TOKEN_SYMBOL = "BG";
    uint256 constant private MAX_TOKEN = 1000;      // 1000個まで販売
    uint256 constant private MAX_MINT_AT_ONCE = 10; // 1回のTxで10個までmint可能

    //--------------------------------------------------------
    // ストレージ
    //--------------------------------------------------------
    uint256 private _sale_price;        // 販売価格
    uint256 private _sale_start;        // 開始時間
    uint256 private _sale_end;          // 終了時間

    // メタデータの参照先のベース
    string private _base_uri_for_metadata;

    uint256 private _total_supply;      // 発行数

    //--------------------------------------------------------
    // コンストラクタ
    //--------------------------------------------------------
    constructor() Ownable() ERC721( TOKEN_NAME, TOKEN_SYMBOL ) {
        // 初期設定
        _sale_price = 10000000000000000;    // 値段:0.1 ETH
        _sale_start = 1660532400;           // 開始:2022 8/15 12:00(JST)
        _sale_end = 0;                      // 終了:売れ切れるまで(0で無期限)

        _base_uri_for_metadata = "ipfs://QmXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX/";  // このサンプルではダミー(本来はここが有効なハッシュ値になる)
    }

    //--------------------------------------------------------
    // [external] 取得
    //--------------------------------------------------------
    function salePrice() external view returns (uint256) { return( _sale_price ); }
    function saleStart() external view returns (uint256) { return( _sale_start ); }
    function saleEnd() external view returns (uint256) { return( _sale_end ); }

    function baseUriForMetadata() external view returns (string memory) { return( _base_uri_for_metadata ); }

    function totalSupply() external view returns (uint256) { return( _total_supply ); }
    function maxSupply() external pure returns (uint256) { return( MAX_TOKEN ); }
    function maxMintAtOnce() external pure returns (uint256) { return( MAX_MINT_AT_ONCE ); }

    //--------------------------------------------------------
    // [external] 設定
    //--------------------------------------------------------
    function setSalePrice( uint256 price ) external onlyOwner { _sale_price = price; }
    function setSaleStart( uint256 start ) external onlyOwner { _sale_start = start; }
    function setSaleEnd( uint256 end ) external onlyOwner { _sale_price = end; }

    function setBaseUriForMetadata( string calldata uri ) external onlyOwner { _base_uri_for_metadata = uri; }

    //--------------------------------------------------------
    // [public/override] tokenURI(メタデータ参照先)
    //--------------------------------------------------------
    function tokenURI( uint256 tokenId ) public view override returns (string memory) {
        return( string( abi.encodePacked( _base_uri_for_metadata, Strings.toString( tokenId ) ) ) );
    }

    //--------------------------------------------------------
    // [external/payable] 購入(mint)
    //--------------------------------------------------------
    function mint( uint256 num ) external payable {
        require( _sale_start == 0 || _sale_start <= block.timestamp, "before sale" );
        require( _sale_end == 0 || _sale_end > block.timestamp, "after sale" );
        require( MAX_MINT_AT_ONCE >= num, "max mint" );
        require( msg.value >= _sale_price, "insufficient value" );

        // 指定個数分MINT
        for( uint256 i=0; i<num; i++ ){
            _safeMint( msg.sender, _total_supply + i );
        }
        _total_supply += num;
    }

    //--------------------------------------------------------
    // [external] 売り上げの引き出し
    //--------------------------------------------------------
    function withdraw() external {
        address payable target = payable( msg.sender );
        target.transfer( address(this).balance );
    }
}

①桁のミス

コンストラクタで販売価格を設定していますが、よく見るとコメント「0.1 ETH」に対してweiの指定が1桁少ないです。

// 誤
_sale_price = 10000000000000000;    // 値段:0.1 ETH

// 正
_sale_price = 100000000000000000;   // 値段:0.1 ETH

販売価格や販売期間等は大事な値です。
特に、価格は「wei」で指定することになるので、桁のミスには気をつけましょう。
ちなみに、block.timestampに対しては unixtime(秒)で指定します(ミリ秒で指定しないように注意)。

②変数の代入ミス

「setSaleEnd」で「_sale_price」変数へ代入してしまっています。

// 誤
function setSaleEnd( uint256 end ) external onlyOwner { _sale_price = end; }

// 正
function setSaleEnd( uint256 end ) external onlyOwner { _sale_end = end; }

ありえないと思うかも知れませんが、関数をコピペで量産する場合にやりがちなミスです。
「setSalePrice」からコードをコピペした際、関数名と引数名をリネームした後、変数名を書き換え忘れたという状況です。

③requireの条件の間違い&記述漏れ

ETHの転送額(msg.value)の判定を、MINT1回分の価格で行ってしまっています。

// 誤
require( msg.value >= _sale_price, "insufficient value" );

// 正
require( msg.value >= num*_sale_price, "insufficient value" );

購入時のミント数の仕様が「1個 → 最大で10個」に変更されたことで、「mint」関数の引数に「num」が追加されたという背景があります。で、その際、まんまと値段の判定を修正し忘れてしまったというわけです。

さらに、もう1点見過ごせないのが「MAX_TOKEN」の定数がどこにも使われていないことです。このNFTプロジェクトはNFTの発行数を「1000」で打ち止めにしたいのに、うっかり、その制限を実装し忘れてしまっています。

本来であれば、「mint」関数に下記のrequireが追加されるべきでした。

require( MAX_TOKEN >= (num+_total_supply), "max token" );

このrequireによる制限がないことで、NFTが1000を超えて発行されてしまう可能性があります(メタデータが1000個分しか用意されていない場合、壊れたNFTがOpenSea上に表示されてしまいます)。

この問題に気がついた運営は、頃合いを見計らってセールを終了(setSaleEnd)しようとするでしょう。

が、不幸なことに、「setSaleEnd」は変数の指定ミスで「_sale_end」変数ではなく「_sale_price」を変えてしまいます!
(setSaleEndを呼んでしまうと、_sale_priceに unixtime相当の値が指定され、とても低い価格で mintされてしまうという不具合の連鎖が引き起こされます)

コードを見直した結果、_sale_endを指定する手段が存在しないことに運営は気づくでしょう。
そして、壊れたNFTが無限にMINTされつづけるのを、ただ眺めることしかできなくなってしまいます。
(実は、_sale_startをとんでもない未来に指定することでMINTは止めることができますが、この時点でプロジェクトの信用はガタ落ちでしょう...)

④アクセス権限の追加漏れ

プロジェクトにとって売り上げの引き出しは最も大事な部分です。
が、よくみると引き出し関数「withdraw」にアクセス制限がかかっておらず、誰でも売り上げを持ち去ることができてしまう状況です。

// 誤
function withdraw() external {
  address payable target = payable( msg.sender );

// 正(アクセス制限の追加)
function withdraw() external onlyOwner {
  address payable target = payable( msg.sender );

// 正(受け取り対象を固定)
function withdraw() external {
  address payable target = payable( owner() );

さすがにこれは極端な例ですが、管理者(onlyOwner等)のみが呼ぶ想定の書き込み処理に、アクセス制限をかけ忘れることで、資産の持ち出しや、各種設定の上書きなど、致命的な不具合が引き起こされます。

動作確認は異常系まで網羅しよう

上にあげたスマコンの不具合ですが、コードの査読で見つからなくても、動作確認をすることで全て検出できます。
デプロイ前、コードをFIXした段階で、下記の目線で動作確認できるのが理想です。

各種設定値は正常か?
販売価格や販売日時等を、get関数から取得し、意図通りの値になっているか確認しましょう。

設定関数を叩いた後、対象の値がちゃんと変わっているか?
全ての「set***」と「get***」の動作を見比べて、ちゃんと設定が反映されるかを確認しましょう。

資産(ETH等)不足でエラーになるか?
ETHを送らない場合や、設定価格よりも少なく送った場合にエラーになるか確認しましょう。

最大値をまたぐトランザクションがエラーになるか?
最大値までの動作検証は大変ですが、こちらも忘れず確認しましょう。
あまりに数が大きい場合は、テスト環境で定数を小さくして問題がないかを確認した後、本番想定の値にもどすやり方が無難でしょう。

アクセス制限を想定する処理を一般アドレスから叩いた場合にエラーなるか?
最低限、資産のやりとり、設定変数の変更をする関数の呼び出しが、管理者以外から行われた場合、エラーになることを確認しましょう。

正常系の動作確認が終わると「スマコンのコーディングは終了!」としたくなるものですが、「脆弱性」の観点からは異常系の動作確認のほうが重要です。「面倒だな」「つまらないな」という気持ちはぐっと押さえて、すべての異常系の動作確認を、最低1度は行うようにしたいところです。

それでも、スケジュール的に十分な時間がとれない場合はあると思います。

そんな時は「資産が奪われないか」、「データへの悪戯を防いでいるか」の2点に集中してコードを読み、うっかりミスぐらいは回避できるように最善を尽くしましょう(設定関数の変数の代入ミスとか、アクセス制限のつけ忘れとか、本当に恥ずかしくて死にたくなりますから...)。

また、常日頃、他人のスマコンを「悪意を持って」読む癖をつけることで、事前確認の精度があがると思います。

では、くどいようですが、最後にもう一度。
「EVM系のスマコンはデプロイしたら終わりです」
だからこそ、デプロイに先立って万全を期しましょう。

補足:Auditの利用を検討しよう

余裕(金銭的&時間的)のあるプロジェクトの場合は「Audit=スマコンの監査」の利用を検討しましょう。第三者(十分な知識をもったスマコンのエキスパート)にコードをチェックしてもらい、脆弱性や非効率な部分の指摘をうけるというものです。

とくに、ミスが絶対に許されないスマコンの実装において、外部の専門家に見てもらえるということは、技術面&精神面で強力な助けになってくれるはずです。

22
9
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
22
9