Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions contracts/FlowIDTableStaking.cdc
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,9 @@ access(all) contract FlowIDTableStaking {
role >= UInt8(1) && role <= UInt8(5): "FlowIDTableStaking.NodeRecord.init: The role must be 1, 2, 3, 4, or 5 but got \(role)"
FlowIDTableStaking.isValidNetworkingAddress(address: networkingAddress): "FlowIDTableStaking.NodeRecord.init: The networkingAddress must be a valid domain name with a port (e.g., node.flow.com:3569), must not exceed 510 characters, and cannot be an IP address, but got \(networkingAddress)"
networkingKey.length == 128: "FlowIDTableStaking.NodeRecord.init: The networkingKey length must be exactly 64 bytes (128 hex characters) but got \(networkingKey.length)"
FlowIDTableStaking.isLowercaseHex(networkingKey): "FlowIDTableStaking.NodeRecord.init: The networkingKey must contain only lowercase hex characters (0-9, a-f)"
stakingKey.length == 192: "FlowIDTableStaking.NodeRecord.init: The stakingKey length must be exactly 96 bytes (192 hex characters) but got \(stakingKey.length)"
FlowIDTableStaking.isLowercaseHex(stakingKey): "FlowIDTableStaking.NodeRecord.init: The stakingKey must contain only lowercase hex characters (0-9, a-f)"
!FlowIDTableStaking.getNetworkingAddressClaimed(address: networkingAddress): "FlowIDTableStaking.NodeRecord.init: The networkingAddress \(networkingAddress) has already been claimed by another node and cannot be used again"
!FlowIDTableStaking.getNetworkingKeyClaimed(key: networkingKey): "FlowIDTableStaking.NodeRecord.init: The networkingKey \(networkingKey) has already been claimed by another node and cannot be used again"
!FlowIDTableStaking.getStakingKeyClaimed(key: stakingKey): "FlowIDTableStaking.NodeRecord.init: The stakingKey \(stakingKey) has already been claimed by another node and cannot be used again"
Expand Down Expand Up @@ -1819,12 +1821,14 @@ access(all) contract FlowIDTableStaking {
}
}

/// Checks if the given string has all numbers or lowercase hex characters
/// Used to ensure that there are no duplicate node IDs
access(all) view fun isValidNodeID(_ input: String): Bool {
/// Checks if the given string contains only lowercase hex characters (0-9, a-f)
/// Used to prevent case-aliasing attacks where different hex strings
/// (e.g., "aabb" vs "AABB") decode to the same bytes
access(all) view fun isLowercaseHex(_ input: String): Bool {
let byteVersion = input.utf8

for character in byteVersion {
// Only allow 0-9 (ASCII 48-57) and a-f (ASCII 97-102)
if ((character < 48) || (character > 57 && character < 97) || (character > 102)) {
return false
}
Expand All @@ -1833,6 +1837,11 @@ access(all) contract FlowIDTableStaking {
return true
}

/// Checks if the given node ID has all numbers or lowercase hex characters
access(all) view fun isValidNodeID(_ input: String): Bool {
return self.isLowercaseHex(input)
}

/// Validates that a networking address is properly formatted
/// Requirements:
/// 1. Must not be an IP address
Expand Down
44 changes: 33 additions & 11 deletions contracts/FlowTransactionScheduler.cdc
Original file line number Diff line number Diff line change
Expand Up @@ -1410,19 +1410,10 @@ access(all) contract FlowTransactionScheduler {
/// checking storage used before and after to see how large the data is in MB
/// If data is nil, the function returns 0.0
access(all) fun getSizeOfData(_ data: AnyStruct?): UFix64 {
if data == nil {
if self.isSizeTrivial(data) {
return 0.0
} else {
let type = data!.getType()
if type.isSubtype(of: Type<Number>())
|| type.isSubtype(of: Type<Bool>())
|| type.isSubtype(of: Type<Address>())
|| type.isSubtype(of: Type<Character>())
|| type.isSubtype(of: Type<Capability>())
{
return 0.0
}
}

let storagePath = /storage/dataTemp
let storageUsedBefore = self.account.storage.used
self.account.storage.save(data!, to: storagePath)
Expand All @@ -1432,6 +1423,37 @@ access(all) contract FlowTransactionScheduler {
return FlowStorageFees.convertUInt64StorageBytesToUFix64Megabytes(storageUsedAfter.saturatingSubtract(storageUsedBefore))
}


access(all) fun isSizeTrivial(_ optionalData: AnyStruct?): Bool {
// If data is non-nil, skip the known small-sized primitive types.
if let data = optionalData {
let type = data.getType()

// Arbitrary-sized numbers can be large.
// So they must be measured.
if type.isSubtype(of: Type<Int>())
|| type.isSubtype(of: Type<UInt>()) {
return false
}

// Some primitive types are trivial in size.
if type.isSubtype(of: Type<Number>())
|| type.isSubtype(of: Type<Bool>())
|| type.isSubtype(of: Type<Address>())
|| type.isSubtype(of: Type<Character>())
|| type.isSubtype(of: Type<Capability>()) {
return true
}

// Everything else needs to be measured.
return false
}

// Data is nil: no need to meassure.
return true
}


access(all) init() {
self.storagePath = /storage/sharedScheduler
let scheduler <- create SharedScheduler()
Expand Down
12 changes: 6 additions & 6 deletions lib/go/contracts/internal/assets/assets.go

Large diffs are not rendered by default.

131 changes: 131 additions & 0 deletions lib/go/test/flow_epoch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import (
"encoding/hex"
"fmt"
"math/rand"
"strings"
"testing"
"time"

"github.com/onflow/cadence"
jsoncdc "github.com/onflow/cadence/encoding/json"
"github.com/onflow/cadence/interpreter"
"github.com/onflow/crypto"
"github.com/onflow/flow-go/module/signature"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -2036,3 +2038,132 @@ func verifyEpochRecoverGovernanceTx(
}
verifyEpochRecover(t, adapter, idTableAddress, expectedRecoverEvent)
}

// TestKeyCaseValidation tests that the contract properly rejects keys containing
// uppercase hex characters. This prevents case-aliasing attacks where different hex
// strings (e.g., "aabb" vs "AABB") decode to the same bytes but would be treated as
// different keys by string-based uniqueness checks.
//
// The fix adds isLowercaseHex() validation to ensure all networking and staking keys
// contain only lowercase hex characters (0-9, a-f).
func TestKeyCaseValidation(t *testing.T) {
b, _, accountKeys, env := newTestSetup(t)

// Create new keys for the epoch account
idTableAccountKey, IDTableSigner := accountKeys.NewWithSigner()

// Deploy full epoch infrastructure (staking, qc, dkg, epoch contracts)
initializeAllEpochContracts(t, b, idTableAccountKey, IDTableSigner, &env,
startEpochCounter,
numEpochViews,
numStakingViews,
numDKGViews,
numClusters,
randomSource,
rewardIncreaseFactor)

// Generate accounts and keys for testing
numNodes := 3
addresses, _, signers := registerAndMintManyAccounts(t, b, env, accountKeys, numNodes)
ids, _, _ := generateNodeIDs(numNodes)
stakingPrivateKeys, stakingPublicKeys, _, networkingPublicKeys := generateManyNodeKeys(t, numNodes)
stakingKeyPOPs := generateManyKeyPOPs(t, stakingPrivateKeys)

var amountToCommit interpreter.UFix64Value = interpreter.NewUnmeteredUFix64ValueWithInteger(1350000)
var committed interpreter.UFix64Value = interpreter.NewUnmeteredUFix64ValueWithInteger(0)

t.Run("Registration with lowercase keys should succeed", func(t *testing.T) {
// All generated keys should be lowercase hex
registerNode(t, b, env,
addresses[0],
signers[0],
ids[0],
"node0.flow.com:3569",
networkingPublicKeys[0], // lowercase
stakingPublicKeys[0], // lowercase
stakingKeyPOPs[0],
amountToCommit,
committed,
1, // collector role
false)
})

t.Run("Registration with uppercase networking key should fail", func(t *testing.T) {
uppercaseNetworkingKey := strings.ToUpper(networkingPublicKeys[1])

// Verify the key is actually uppercase
require.NotEqual(t, networkingPublicKeys[1], uppercaseNetworkingKey,
"uppercase key should be different from original")

// Verify both decode to the same bytes (demonstrating the attack vector)
bytes1, err := hex.DecodeString(networkingPublicKeys[1])
require.NoError(t, err)
bytes2, err := hex.DecodeString(uppercaseNetworkingKey)
require.NoError(t, err)
require.Equal(t, bytes1, bytes2,
"both keys should decode to identical bytes")

// Registration should fail because uppercase hex is rejected
registerNode(t, b, env,
addresses[1],
signers[1],
ids[1],
"node1.flow.com:3569",
uppercaseNetworkingKey, // UPPERCASE - should be rejected
stakingPublicKeys[1], // lowercase
stakingKeyPOPs[1],
amountToCommit,
committed,
2, // consensus role
true) // should fail
})

t.Run("Registration with uppercase staking key should fail", func(t *testing.T) {
uppercaseStakingKey := strings.ToUpper(stakingPublicKeys[2])

// Verify the key is actually uppercase
require.NotEqual(t, stakingPublicKeys[2], uppercaseStakingKey,
"uppercase key should be different from original")

// Registration should fail because uppercase hex is rejected
registerNode(t, b, env,
addresses[2],
signers[2],
ids[2],
"node2.flow.com:3569",
networkingPublicKeys[2], // lowercase
uppercaseStakingKey, // UPPERCASE - should be rejected
stakingKeyPOPs[2],
amountToCommit,
committed,
3, // execution role
true) // should fail
})

t.Run("Registration with mixed case networking key should fail", func(t *testing.T) {
// Create a mixed case key (e.g., "aAbBcCdD...")
originalKey := networkingPublicKeys[1]
mixedCaseKey := ""
for i, c := range originalKey {
if i%2 == 0 {
mixedCaseKey += strings.ToUpper(string(c))
} else {
mixedCaseKey += string(c)
}
}

// Registration should fail because mixed case hex is rejected
registerNode(t, b, env,
addresses[1],
signers[1],
ids[1],
"node1b.flow.com:3569",
mixedCaseKey, // MIXED CASE - should be rejected
stakingPublicKeys[1], // lowercase
stakingKeyPOPs[1],
amountToCommit,
committed,
2, // consensus role
true) // should fail
})
}
11 changes: 9 additions & 2 deletions tests/transactionScheduler_test.cdc
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,17 @@ access(all) fun testInit() {
access(all) fun testGetSizeOfData() {

// Test different values for data to verify that it reports the correct sizes

var size = getSizeOfData(data: 1)
Test.assertEqual(0.00000000 as UFix64, size)
Test.assertEqual(0.00002300 as UFix64, size)

size = getSizeOfData(data: 100000000)
Test.assertEqual(0.00002600 as UFix64, size)

size = getSizeOfData(data: Int8(1))
Test.assertEqual(0.00000000 as UFix64, size)

size = getSizeOfData(data: UInt256(100000000))
Test.assertEqual(0.00000000 as UFix64, size)

size = getSizeOfData(data: StoragePath(identifier: "scheduledTransactionsStoragePath"))
Expand All @@ -82,7 +89,7 @@ access(all) fun testGetSizeOfData() {
size = getSizeOfData(data: testData)
Test.assertEqual(0.00003000 as UFix64, size)

size = getSizeOfData(data: 0x0000000000000001)
size = getSizeOfData(data: Address(0x0000000000000001))
Test.assertEqual(0.00000000 as UFix64, size)

let largeArray: [Int] = []
Expand Down
Loading