LoginSignup
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

@flowcadence

Cadence Anti-Patterns

Last updated at Posted at 2024-10-20

Previous << Design Patterns
Next >> Development Standards


これは、本番環境向けのCadenceコードで発見された場合、改善できる問題の意見の多いリストです。

Avoid using fully authorized account references as a function parameter

Problem

開発者は、ユーザーのアカウント参照またはアドレスを使用して、ユーザーの認証や操作を行うことができます。これを行うには、認証済みアカウント参照型を持つ関数にパラメータを追加します（auth(...) &Account）。認証済みアカウント参照は、トランザクションに署名することでのみ取得できるためです。

これが完全に認証されたアカウント参照である場合、これは問題となります。なぜなら、完全に認証されたアカウント参照は、アカウント上のいくつかの機密性の高い操作、例えばストレージへの書き込みなどへのアクセスを許可するからです。これは、悪意のあるユーザーに悪用される可能性があることを意味します。

Example:

...
/* BAD CODE
   DO NOT COPY */

/* Imagine this code is in a contract that uses a `auth(Storage) &Account` parameter
 * to authenticate users to transfer NFTs */

/* They could deploy the contract with an Ethereum-style access control list */ functionality

access(all)
fun transferNFT(id: UInt64, owner: auth(Storage) &Account) {
    assert(owner(id) == owner.address)

    transfer(id)
}

/* But they could upgrade the function to have the same signature
 * so it looks like it is doing the same thing, but they could also drain a little bit
 * of FLOW from the user's vault, a totally separate piece of the account that
 * should not be accessible in this function
 * BAD */

access(all)
fun transferNFT(id: UInt64, owner: auth(Storage) &Account) {
    assert(owner(id) == owner.address)

    transfer(id)

    /* Sneakily borrow a reference to the user's Flow Token Vault
     * and withdraw a bit of FLOW
     * BAD */
    let vaultRef = owner.borrow<&FlowToken.Vault>(/storage/flowTokenVault)!
    let stolenTokens <- vaultRef.withdraw(amount: 0.1)

    /* deposit the stolen funds in the contract owners vault
       BAD */
    contractVault.deposit(from: <-stolenTokens)
}
...

Solution

プロジェクトは、リソースやCapabilityを認証オブジェクトとして使用するなど、ユーザーを認証する他の方法を見つけるべきです。また、ストレージやリンク操作のほとんどは、コントラクトユーティリティ関数内ではなく、トランザクション本体内で実行することを想定すべきです。

コールドストレージのマルチシグなど、認証済みアカウント参照（auth(...) &Account）を使用する必要があるシナリオもありますが、そのようなケースはまれであり、必要なアカウントの機能(account functionality)が非常に制限されたサブセットである場合にのみ使用すべきです。

Public functions and fields should be avoided

Problem

コードを構築する際には、アクセス修飾子を常に把握し、公開すべきもののみを公開するようにしてください。誤って公開されたフィールドは、セキュリティ上の脆弱性となり得ます。

Solution

スマートコントラクトを記述する際には、すべてのフィールドと関数を確認し、Entitlement（access(E)）によるアクセスが必要であることを確認するか、または、必要でない限り、access(self)、access(contract)、またはaccess(account)などのnon-public アクセス修飾子を使用してください。

Capability-Typed public fields are a security hole

これは、上記の「パブリック関数およびフィールドは避けるべき」の特別なケースです。

Problem

パブリックフィールドの値はコピーすることができます。 Capabilityは値型なので、パブリックフィールドとして使用されている場合、誰でもフィールドからコピーして、そのCapabilityが公開している関数を呼び出すことができます。 コントラクトまたはリソースのフィールドとしてこの方法でCapabilityが保存されている場合、これはおそらく望む結果ではないでしょう。

Solution

Capabilityへのパブリックアクセスを可能にするには、この期待結果が明確になるよう、アカウントのパブリックエリアに配置します。

Public admin resource creation functions are unsafe

これは、上記の「パブリック関数およびフィールドは避けるべき」の特別なケースです。

Problem

リソースを作成するスマートコントラクト上のパブリック関数は、どのアカウントでも呼び出すことができます。そのリソースが管理機能へのアクセスを提供している場合、作成関数はパブリックにすべきではありません。

Solution

これを修正するには、スマートコントラクトの初期化時にそのリソースの単一インスタンスを作成し、必要に応じて、新しい作成関数を管理リソース内に含めることができます。

Example

/* 疑似コード */

/* BAD */
access(all)
contract Currency {

    access(all)
    resource Admin {

        access(all)
        fun mintTokens()
    }

    /* Anyone in the network can call this function
       And use the Admin resource to mint tokens */
    access(all)
    fun createAdmin(): @Admin {
        return <-create Admin()
    }
}

/* This contract makes the admin creation private and in the initializer
 * so that only the one who controls the account can mint tokens
 * GOOD */
access(all)
contract Currency {

    access(all)
    resource Admin {

        access(all)
        fun mintTokens()

        /* Only an admin can create new Admins */
        access(all)
        fun createAdmin(): @Admin {
            return <-create Admin()
        }
    }

    init() {
        /* Create a single admin resource */
        let firstAdmin <- create Admin()

        /* Store it in private account storage, so only the admin can use it */
        self.account.storage.save(<-firstAdmin, to: /storage/currencyAdmin)
    }
}

Do not modify smart contract state or emit events in public struct initializers

これは、スマートコントラクトに一般公開されたアクセス可能な部分があることのリスクを示すもう一つの例です。

Problem

現在、Cadenceにおけるデータ構造の定義(原文: Data structure definitions)は、誰でも使用できるように「パブリック」として宣言しなければなりません。構造体にはリソースのような制限がないため、認証(authorization)手続きを経ることなく誰でも新しい構造体のインスタンスを作成することができます。

Solution

構造体の作成に関連する、状態を変更する操作はすべて、構造体のイニシャライザー(init関数)ではなくリソースに格納すべきである。

Example

これはかつてはNBA Top Shotスマートコントラクトのバグでした。これを例として説明します。以前は、新しい(NBAの)プレイが作成されると、(NBAの)プレイのレコードが構造体で初期化され、プレイIDを追跡する数値がインクリメントされ、イベントが発行されていました。

/* Simplified Code
 * BAD
 */
access(all)
contract TopShot {

    /* The Record that is used to track every unique play ID */
    access(all)
    var nextPlayID: UInt32

    access(all)
    struct Play {

        access(all)
        let playID: UInt32

        init() {

            self.playID = TopShot.nextPlayID

            /* Increment the ID so that it isn't used again */
            TopShot.nextPlayID = TopShot.nextPlayID + 1

            emit PlayCreated(id: self.playID, metadata: metadata)
        }
    }
}

これはリスクです。なぜなら、誰でも好きなだけPlay構造体を生成することができ、nextPlayIDフィールドが最大UInt32値まで増加し、事実上、新しいプレイが作成できなくなる可能性があるからです。また、偽のイベントが発行されることにもなります。

このバグは、代わりにプレイを作成する管理機能(admin function)でコントラクトのstateを更新することで修正されました。

/* Update contract state in admin resource functions
 * GOOD
 */
access(all)
contract TopShot {

    /* The Record that is used to track every unique play ID */
    access(all)
    var nextPlayID: UInt32

    access(all)
    struct Play {

        access(all)
        let playID: UInt32

        init() {
            self.playID = TopShot.nextPlayID
        }
    }

    access(all)
    resource Admin {

        /* Protected within the private admin resource */
        access(all)
        fun createPlay() {
            /* Create the new Play */
            var newPlay = Play()

            /* Increment the ID so that it isn't used again */
            TopShot.nextPlayID = TopShot.nextPlayID + UInt32(1)

            emit PlayCreated(id: newPlay.playID, metadata: metadata)

            /* Store it in the contract storage */
            TopShot.playDatas[newPlay.playID] = newPlay
        }
    }
}

翻訳元

Previous << Design Patterns

Flow BlockchainのCadence version1.0ドキュメント (Cadence Anti-Patterns)

Next >> Development Standards

0
0
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
Sign upLogin
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?