Account Abstraction Schnorr Signatures SDK - InFlux Technologies

The codebase used in different code points a predictable salt value (const salt = "this is salt"), which could lead to a collision attack risk.

A predictable salt could allow an attacker to precompute address collisions if they can anticipate the public keys used during contract deployments. This could result in unauthorized address generation, undermining the integrity of the smart account creation process.

Impact

Using a predictable salt in the smart account creation process increases the risk of:

• Address Collision Attacks: An attacker could precompute the same address, potentially overriding a legitimate deployment.

• Unauthorized Asset Access: If an address collision occurs, funds could be diverted or compromised.

• Smart Contract Integrity Compromise: The deterministic nature of the salt could allow unauthorized access to smart accounts with predictable addresses.

Listed below are some examples where the “salt” implementation was detected to be potentially insecure.

• examples/account-address/account_address.ts

function getAddressOffChain(combinedAddresses: string[], salt: string) {
  const factorySalt = "aafactorysalt"
  const factoryAddress = deployments[polygon.id]?.MultiSigSmartAccountFactory

• examples/account-address/account_address.ts

const combinedAddresses = getAllCombinedAddrFromKeys(publicKeys, 3)
const salt = "random salt for randomly generated priv keys"

• examples/account-deployment/factory-create-account-method-call-deployment.ts

let privKey2
do privKey2 = randomBytes(32)
while (!secp256k1.privateKeyVerify(privKey2))
const schnorrSigner2 = createSchnorrSigner(privKey2)
const publicKey2 = schnorrSigner2.getPubKey()

const salt = "this is salt"

• examples/sign_3_of_3/sign-3_of_3.ts

/**
 * Multi Sig participant #3
 */
let privKey3
do privKey3 = randomBytes(32)
while (!secp256k1.privateKeyVerify(privKey3))
const schnorrSigner3 = createSchnorrSigner(privKey3)

const publicKey3 = schnorrSigner3.getPubKey()

/**
 * Participants select SALT for the Smart Account
 * (Same 3 Participants can have multiple smart account's where id of those account is SALT)
 */

const salt = "this is salt shared by participants 3"

• src/accountAbstraction/multiSigSmartAccount.ts

export async function createMultiSigSmartAccount({
  transport,
  chain,
  entryPoint = getEntryPoint(chain, { version: "0.6.0" }),
  accountAddress,
  combinedAddress = [],
  salt: _salt,
}: CreateMultiSigSmartAccountParams): Promise<MultiSigSmartAccount> {
  const client = createBundlerClient({
    transport,
    chain,
  })
  const salt = _salt ?? ethers.encodeBytes32String("salt")

To mitigate the predictable salt issue:

• Avoid Hardcoded Values: Ensure salts are dynamically generated and not hardcoded in the source code.

• Use Cryptographically Secure Random Salts: Generate salts using secure randomness (randomBytes() or ethers.utils.randomBytes).

  const salt = randomBytes(32).toString("hex")

• Incorporate User Input: Optionally, derive the salt from both user-specific data and secure random values.

NOT APPLICABLE: Finally agreed with the InFlux Technologies team that this issue was not applicable.

The optional parameter hashFn defaulted to null, relying on external callers to define the hash function, which introduced a cryptographic risk of using an insecure hash.

Impact

If an insecure hash function (e.g., MD5) is provided or null is used, it can compromise the integrity of the signatures.

Listed below, there are some examples of code snippets where the Hash Function was set to null by default.

• src/signers/SchnorrSigner.ts

signMessage(msg: string, hashFn: HashFunction | null = null): SignatureOutput {
    return Schnorrkel.sign(this.#privateKey, msg, hashFn)
  }

• src/signers/Schnorrkel.ts

multiSigSign(
    privateKey: Key,
    msg: string,
    publicKeys: Key[],
    publicNonces: PublicNonces[],
    hashFn: HashFunction | null = null
  ): SignatureOutput {
    const combinedPublicKey = Schnorrkel.getCombinedPublicKey(publicKeys)
    const mappedPublicNonce = this.getMappedPublicNonces(publicNonces)
    const mappedNonces = this.getMappedNonces()

    const musigData = _multiSigSign(
      mappedNonces,
      combinedPublicKey.buffer,
      privateKey.buffer,
      msg,
      publicKeys.map((key) => key.buffer),
      mappedPublicNonce,
      hashFn
    )

• src/signers/Schnorrkel.ts

static verify(
    signature: SchnorrSignature,
    msg: string,
    finalPublicNonce: FinalPublicNonce,
    publicKey: Key,
    hashFn: HashFunction | null = null
  ): boolean {
    return _verify(signature.buffer, msg, finalPublicNonce.buffer, publicKey.buffer, hashFn)
  }

• Set a cryptographically secure default hash function, such as SHA-256:

const defaultHashFunction = (msg: string) => createHash("sha256").update(msg).digest();

SOLVED: The InFlux Technologies team solved this issue by removing the null value and also by adding the "_hashMessage" (solidityPackedKeccak256) as default.

Sensitive information, such as API keys and private keys, were being stored directly in environmental variables.

While environment variables are commonly used to configure applications, storing highly sensitive data in them poses security risks.

Unauthorized users or malicious insiders who gain access to the system or CI/CD pipeline may retrieve these sensitive variables, leading to potential data leaks, unauthorized access to external services, and compromised application security.

The repository code used sensitive private keys stored in a .env file, which are accessed programmatically via process.env.PRIVATE_KEY clauses or similar.

Since the repository is public, there is also a risk that developers may inadvertently copy these sensitive values directly into production environments. This practice can lead to unauthorized access and compromise of digital assets. If environment variables are not properly managed or are leaked, it can result in exposure of private keys.

Impact

Storing private keys or other sensitive information inside environment variables can lead to severe security risks in case of exposure, such as:

• Unauthorized access to blockchain wallets or smart contract deployments.

• Financial loss due to stolen assets.

• Compromise of cryptographic keys, leading to broader security breaches.

Listed below are some examples where sensitive information was being stored inside environmental variables.

• examples/account-deployment/factory-create-account-method-call-deployment.ts

async function factoryCallCreateSmartAccount() {
  const privKey1 = process.env.PRIVATE_KEY as Hex
  const schnorrSigner1 = createSchnorrSigner(privKey1)
  const publicKey1 = schnorrSigner1.getPubKey()

  let privKey2
  do privKey2 = randomBytes(32)
  while (!secp256k1.privateKeyVerify(privKey2))
  const schnorrSigner2 = createSchnorrSigner(privKey2)
  const publicKey2 = schnorrSigner2.getPubKey()

  const salt = "this is salt"

  const publicKeys = [publicKey1, publicKey2]

  const combinedAddresses = getAllCombinedAddrFromKeys(publicKeys, 2)

  const provider = new JsonRpcProvider(process.env.ALCHEMY_RPC_URL)
  const wallet = new Wallet(process.env.PRIVATE_KEY, provider)

  const factoryAddress = deployments[CHAIN.id]?.MultiSigSmartAccountFactory

  const smartAccountAddress = await predictAccountAddrOnchain(factoryAddress, combinedAddresses, salt, provider)
  console.log("Smart Account Address:", smartAccountAddress)

• examples/sign_3_of_3/sign-3_of_3.ts

async function main() {
  /**
   * Wallet to cover initial transaction costs/prefund smart account
   */
  const provider = new JsonRpcProvider(process.env.ALCHEMY_RPC_URL)
  const wallet = new Wallet(process.env.PRIVATE_KEY, provider)

  /**
   * Precondition:
   * 3 Participants sharing they Public Key's
   */

  /**
   * Multi Sig participant #1
   */
  const privKey1 = process.env.PRIVATE_KEY as Address
  const schnorrSigner1 = createSchnorrSigner(privKey1)

  const publicKey1 = schnorrSigner1.getPubKey()

• examples/user-operation/transfer-native/transfer-native.ts

async function main() {
  /**
   * Wallet to cover initial transaction costs/prefund smart account
   */
  const provider = new JsonRpcProvider(process.env.ALCHEMY_RPC_URL)
  const wallet = new Wallet(process.env.PRIVATE_KEY, provider)

  /**
   * Requirements
   * Eth/Matic: 0.06
   */

  const walletBalance = await provider.getBalance(wallet.address)
  if (walletBalance < parseEther("0.06")) throw new Error("Not enough native assets")

  const privKey1 = process.env.PRIVATE_KEY as Address
  const schnorrSigner1 = createSchnorrSigner(privKey1)

  const publicKey1 = schnorrSigner1.getPubKey()

• Remove sensitive keys from .env files in public repositories.

• Use environment-specific secrets management tools (e.g., AWS Secrets Manager, HashiCorp Vault or hardware key management (HSM)).

• Ensure the .env.example file includes placeholders or comments explaining proper key usage, without actual secrets.

• Implement CI/CD checks to prevent committing sensitive data to public repositories.

RISK ACCEPTED: According to the InFlux Technologies team, they wanted to accept the risk of this issue.

The scoped repository used multiple third-party dependencies. Several of the assessed dependencies had public-known vulnerabilities, many of them with CRITICAL or HIGH severity, that may pose a high risk to the global application security level.

Impact

Using vulnerable third-party libraries can result in security vulnerabilities in the project that can be exploited by attackers. This can result in data breaches, theft of sensitive information, and other security issues.

• snyk test --all-projects command output.

Update all affected packages to its latest version.

It is strongly recommended to perform an automated analysis of the dependencies from the birth of the project and if they contain any security issues. Developers should be aware of this and apply any necessary mitigation measures to protect the affected application.

RISK ACCEPTED: According to the InFlux Technologies team, they wanted to accept the risk of this issue.

The method getCombinedPublicKey combined public keys without validating whether they were valid points on the curve. This may allow a rogue-key attack, where a malicious signer can construct a key pair that reveals the private key.

In a multi-signature scheme with aggregate signatures, several participants combine their public keys and collectively sign a message. The rogue-key attack occurs when a malicious attacker introduces a manipulated public key that invalidates the security of the scheme.

In this case, if a malicious actor managed to provide a rogue key during the execution process of the whole multi-signature scheme, the attack would be successful as there was no validation on the keys.

Impact

A rogue participant could manipulate the key combination process, potentially revealing the private key of honest signers.

• Total security breach: The attacker can sign alone on behalf of the entire group.

• Unilateral control of funds: In the context of smart contracts or multi-sig wallets, this could allow the attacker to transfer funds without the cooperation of the rest of the participants.

• src/signers/Schnorrkel.ts

static getCombinedPublicKey(publicKeys: Key[]): Key {
    if (publicKeys.length < 2) throw new Error("At least 2 public keys should be provided")

    const bufferPublicKeys = publicKeys.map((publicKey) => publicKey.buffer)
    const L = _generateL(bufferPublicKeys)

    const modifiedKeys = bufferPublicKeys.map((publicKey) => {
      return secp256k1.publicKeyTweakMul(publicKey, _aCoefficient(publicKey, L))
    })

    return new Key(Buffer.from(secp256k1.publicKeyCombine(modifiedKeys)))
  }

• Validate that all public keys are valid points on the secp256k1 curve before combining.

• Reject zero keys and invalid points explicitly.

• Example Fix:

if (!secp256k1.publicKeyVerify(publicKey)) {
   throw new Error("Invalid public key provided");
}

To prevent a Rogue Key Attack:

• Explicit verification of public keys: Each participant must validate the public keys of the others before combining them.

• Proof-of-knowledge protocols: Implement a scheme such as MuSig2 or MuSig-DN, which requires proof of knowledge of the associated private key before aggregation.

SOLVED: The InFlux Technologies team followed Halborn recommendations to solve this issue.

The initTransactionCost variable was hardcoded in the source code examples, making it static and unresponsive to real-time gas price fluctuations on the blockchain network. This approach did not account for dynamic network conditions, potentially leading to incorrect transaction execution or unexpected failures due to insufficient gas provision.

Impact

The major security concern about this issue was the use of a hardcoded transaction value (initTransactionCost), which could lead to incorrect execution or gas manipulation. This value must be calculated dynamically.

• Economic Denial of Service (EDoS): If gas prices surge unexpectedly, a hardcoded gas limit may lead to transaction failures or delays.

• Manipulation Risk: Attackers could deliberately increase network congestion, forcing the hardcoded value to be insufficient, causing critical operations to fail.

• Reduced Flexibility: The contract cannot adapt to real-time gas price changes, increasing operational risk.

This vulnerability severity was lowered from HIGH to MEDIUM because all the hardcoded values for the transaction cost were found in the examples folder of the project. However, this bad practice should be removed, according to the best security practices.

• examples/account-deployment/user-operation-init-code-deployment.ts

const smartAccountAdddress = await predictAccountAddrOnchain(factoryAddress, combinedAddresses, salt, provider)
  console.log("Smart Account Address:", smartAccountAdddress)

  const initTransactionCost = parseUnits("0.05", 18)
  const addBalanceToSmartAccountTransaction = await wallet.sendTransaction({ to: smartAccountAdddress, value: initTransactionCost })
  await addBalanceToSmartAccountTransaction.wait()

• examples/account-deployment/user-operation-init-code-deployment.ts

const smartAccountAdddress = await predictAccountAddrOnchain(factoryAddress, combinedAddresses, salt, provider)
  console.log("Smart Account Address:", smartAccountAdddress)

  const initTransactionCost = parseUnits("0.05", 18)
  const addBalanceToSmartAccountTransaction = await wallet.sendTransaction({ to: smartAccountAdddress, value: initTransactionCost })
  await addBalanceToSmartAccountTransaction.wait()

• examples/sign_3_of_3/sign-3_of_3.ts

/**
   * Prefund smart account
   */
  const initTransactionCost = parseUnits("0.05", 18)
  const addBalanceToSmartAccountTransaction = await wallet.sendTransaction({ to: multiSigSmartAccount.address, value: initTransactionCost })
  await addBalanceToSmartAccountTransaction.wait()

• examples/user-operation/transfer-erc20/transfer-erc20.ts

/**
  * Prefund smart account
  */
 const initTransactionCost = parseUnits("0.05", 18)
 const addBalanceToSmartAccountTransaction = await wallet.sendTransaction({ to: multiSigSmartAccount.address, value: initTransactionCost })
 await addBalanceToSmartAccountTransaction.wait()

• examples/user-operation/transfer-native/transfer-native.ts

/**
   * Prefund smart account
   */
  const initTransactionCost = parseUnits("0.06", 18)
  const addBalanceToSmartAccountTransaction = await wallet.sendTransaction({ to: multiSigSmartAccount.address, value: initTransactionCost })
  await addBalanceToSmartAccountTransaction.wait()

Similarly to other functions, consider relying on calls to specific contracts to determine the gas estimation. Alternatively, consider using APIs (e.g., blockchain node RPC endpoints or third-party services) as this is the more common approach for gas estimation.

• Replace the hardcoded initTransactionCost with a dynamic calculation based on the current gas price using on-chain data or a reliable oracle.

• Implement a buffer margin to account for sudden price spikes in congested conditions.

• Test the contract under varying gas price scenarios to ensure resilience.

NOT APPLICABLE: Finally agreed with the InFlux Technologies team that this issue was not applicable.

The KeyPair class exposed the privateKey in the toJson() method, which could potentially serialize and expose sensitive private key information to any consumer of this function.

Although it was not invoked in the assessed version, its presence in the code represented a potential risk, particularly if the code is modified in future versions to call it or if the method is inadvertently exposed to insecure channels or logging systems.

• src/types/key-pair.ts

firstnumber=24
toJson(): string {
  return JSON.stringify({
    publicKey: this.publicKey.toHex(),
    privateKey: this.privateKey.toHex(),
  })
}

• Remove all the potential insecure methods that are not being used.

• Avoid Serializing Sensitive Data: Do not expose private keys or sensitive information in the toJson() method or any other serialization mechanism.

• Limit Access to Private Keys: Private keys should only be used within secure contexts and should never be serialized or exposed through external interfaces.

RISK ACCEPTED: According to the InFlux Technologies team, they wanted to accept the risk of this issue.

The Schnorrkel class stored nonces in a private field #nonces. Although nonces were cleared after use with the clearNonces method, the process relied on correct execution flow. If nonces were reused or not properly cleared, it may lead to private key leakage.

Impact

Reusing nonces in Schnorr signatures can completely reveal the private key due to the mathematical properties of the algorithm.

• src/signers/Schnorrkel.ts

export class Schnorrkel {
  #nonces: Nonces = {}

  private _setNonce(privateKey: Buffer): string {
    const { publicNonceData, privateNonceData, hash } = _generatePublicNonces(privateKey)

    const mappedPublicNonce: PublicNonces = {
      kPublic: new Key(Buffer.from(publicNonceData.kPublic)),
      kTwoPublic: new Key(Buffer.from(publicNonceData.kTwoPublic)),
    }

    const mappedPrivateNonce: Pick<NoncePairs, "k" | "kTwo"> = {
      k: new Key(Buffer.from(privateNonceData.k)),
      kTwo: new Key(Buffer.from(privateNonceData.kTwo)),
    }

    this.#nonces[hash] = { ...mappedPrivateNonce, ...mappedPublicNonce }
    return hash
  }

private clearNonces(privateKey: Key): void {
    const x = privateKey.buffer
    const hash = _hashPrivateKey(x)

    // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
    delete this.#nonces[hash]
  }

• Implement a nonce-tracking mechanism to prevent reuse explicitly, even if an error occurs during the signing process.

• Consider enforcing a policy where each nonce is stored in a non-reusable bucket system.

SOLVED: The InFlux Technologies team solved this issue by adding a final block with more cleaning and also a general bucket to remember all nonces used added as well.

The external schnorrkel library was not being used. Instead, the Schnorr implementation was based on the Schnorrkel class defined within the project itself. secp256k1 version 5.0.1 was being used.

As mentioned, secp256k1 is being used for Schnorr signatures. Although functional, the schnorrkel library is often used with Curve25519 for better multi-signature capabilities and robustness and security.

This is a point to review if the security standard of the project requires a higher level of security against quantum attacks.

Impact

While not a direct vulnerability, secp256k1 has a weaker theoretical resistance to rogue attacks compared to Curve25519.

• Consider migrating to the Curve25519 or a library specifically designed for Schnorr signatures such as dalek or schnorrkel.

ACKNOWLEDGED: According to the InFlux Technologies team: "Acknowledged, great suggestion. However, we won't be migrating to it now or anytime soon.".