Skip to content

fix(txscript): allocate fresh slice in opcodeInvert to avoid aliasing#777

Merged
ordishs merged 3 commits into
bsv-blockchain:mainfrom
ordishs:security/4576-opinvert-immutable
May 26, 2026
Merged

fix(txscript): allocate fresh slice in opcodeInvert to avoid aliasing#777
ordishs merged 3 commits into
bsv-blockchain:mainfrom
ordishs:security/4576-opinvert-immutable

Conversation

@ordishs

@ordishs ordishs commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

Closes #4576.

Summary

opcodeInvert used PeekByteArray(0) and XOR'd bytes in place. After OP_DUP (which copies only the slice header), both stack items share the same backing array — so OP_DUP … OP_INVERT mutates the duplicate as well. This diverges from Bitcoin Core / SV Node script semantics.

Fix

Pop, allocate a fresh slice, invert into the new slice, push. Stack transformation a -> ~a is unchanged; the result is now an independent slice with no aliasing.

Test plan

  • Regression test: push a byte slice, push again (aliased), invert top, assert the second copy on the stack is unchanged.
  • Basic correctness: invert known bytes, assert top of stack matches bitwise-NOT.
  • go test -race ./services/legacy/txscript/... passes.

@github-actions

github-actions Bot commented Apr 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🤖 Claude Code Review

Status: Complete

Current Review:
No issues found. The fix correctly addresses the slice aliasing bug by allocating a fresh backing array.

Analysis:

The change properly fixes the aliasing vulnerability in opcodeInvert. The original implementation used PeekByteArray(0) and mutated bytes in-place, which caused problems when the top stack item shared a backing array with another stack item (e.g., after OP_DUP).

Fix verification:

  • Pattern now matches opcodeAnd, opcodeOr, and opcodeXor (all use Pop → allocate → operate → Push)
  • Test coverage is comprehensive: aliasing regression test + basic correctness cases
  • Minimal change scope: only the buggy function modified
  • Stack semantics preserved: a -> ~a transformation unchanged

@github-actions

github-actions Bot commented Apr 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Benchmark Comparison Report

Baseline: main (unknown)

Current: PR-777 (720ad0f)

Summary

  • Regressions: 0
  • Improvements: 0
  • Unchanged: 144
  • Significance level: p < 0.05
All benchmark results (sec/op)
Benchmark Baseline Current Change p-value
_NewBlockFromBytes-4 1.584µ 1.592µ ~ 0.700
SplitSyncedParentMap_SetIfNotExists/256_buckets-4 71.08n 71.36n ~ 0.200
SplitSyncedParentMap_SetIfNotExists/16_buckets-4 71.68n 71.30n ~ 0.700
SplitSyncedParentMap_SetIfNotExists/1_bucket-4 70.95n 71.23n ~ 0.100
SplitSyncedParentMap_ConcurrentSetIfNotExists/256_buckets... 31.87n 32.08n ~ 1.000
SplitSyncedParentMap_ConcurrentSetIfNotExists/16_buckets_... 53.96n 54.34n ~ 1.000
SplitSyncedParentMap_ConcurrentSetIfNotExists/1_bucket_pa... 127.4n 139.7n ~ 0.200
MiningCandidate_Stringify_Short-4 216.6n 217.4n ~ 0.300
MiningCandidate_Stringify_Long-4 1.608µ 1.606µ ~ 0.600
MiningSolution_Stringify-4 842.0n 841.6n ~ 1.000
BlockInfo_MarshalJSON-4 1.706µ 1.722µ ~ 0.100
NewFromBytes-4 123.6n 125.3n ~ 0.100
AddTxBatchColumnar_Validation-4 2.551µ 2.727µ ~ 0.200
OffsetValidationLoop-4 547.9n 556.8n ~ 0.200
Mine_EasyDifficulty-4 65.90µ 65.32µ ~ 0.400
Mine_WithAddress-4 6.907µ 6.991µ ~ 0.200
DirectSubtreeAdd/4_per_subtree-4 61.05n 58.58n ~ 0.200
DirectSubtreeAdd/64_per_subtree-4 31.36n 32.12n ~ 0.200
DirectSubtreeAdd/256_per_subtree-4 30.60n 30.31n ~ 0.700
DirectSubtreeAdd/1024_per_subtree-4 29.06n 29.05n ~ 0.600
DirectSubtreeAdd/2048_per_subtree-4 28.67n 28.70n ~ 0.600
SubtreeProcessorAdd/4_per_subtree-4 292.2n 283.0n ~ 0.200
SubtreeProcessorAdd/64_per_subtree-4 281.3n 283.8n ~ 0.600
SubtreeProcessorAdd/256_per_subtree-4 287.2n 282.4n ~ 0.800
SubtreeProcessorAdd/1024_per_subtree-4 275.1n 271.3n ~ 0.100
SubtreeProcessorAdd/2048_per_subtree-4 276.2n 273.9n ~ 0.100
SubtreeProcessorRotate/4_per_subtree-4 280.8n 282.7n ~ 0.200
SubtreeProcessorRotate/64_per_subtree-4 278.2n 280.9n ~ 0.700
SubtreeProcessorRotate/256_per_subtree-4 276.6n 276.5n ~ 1.000
SubtreeProcessorRotate/1024_per_subtree-4 276.6n 276.0n ~ 0.700
SubtreeNodeAddOnly/4_per_subtree-4 54.35n 54.64n ~ 1.000
SubtreeNodeAddOnly/64_per_subtree-4 34.18n 34.39n ~ 0.400
SubtreeNodeAddOnly/256_per_subtree-4 33.36n 33.45n ~ 0.700
SubtreeNodeAddOnly/1024_per_subtree-4 32.67n 32.70n ~ 0.800
SubtreeCreationOnly/4_per_subtree-4 114.0n 113.8n ~ 1.000
SubtreeCreationOnly/64_per_subtree-4 396.4n 399.2n ~ 1.000
SubtreeCreationOnly/256_per_subtree-4 1.328µ 1.349µ ~ 0.100
SubtreeCreationOnly/1024_per_subtree-4 4.404µ 4.445µ ~ 0.600
SubtreeCreationOnly/2048_per_subtree-4 8.185µ 8.171µ ~ 1.000
SubtreeProcessorOverheadBreakdown/64_per_subtree-4 275.0n 275.7n ~ 0.700
SubtreeProcessorOverheadBreakdown/1024_per_subtree-4 277.9n 277.4n ~ 0.700
ParallelGetAndSetIfNotExists/1k_nodes-4 2.270m 2.210m ~ 0.100
ParallelGetAndSetIfNotExists/10k_nodes-4 5.411m 5.291m ~ 0.100
ParallelGetAndSetIfNotExists/50k_nodes-4 7.635m 7.259m ~ 0.200
ParallelGetAndSetIfNotExists/100k_nodes-4 10.50m 10.12m ~ 0.200
SequentialGetAndSetIfNotExists/1k_nodes-4 2.001m 1.955m ~ 0.100
SequentialGetAndSetIfNotExists/10k_nodes-4 4.437m 4.388m ~ 0.200
SequentialGetAndSetIfNotExists/50k_nodes-4 12.77m 12.28m ~ 0.100
SequentialGetAndSetIfNotExists/100k_nodes-4 22.73m 22.40m ~ 0.200
ProcessOwnBlockSubtreeNodesParallel/1k_nodes-4 2.264m 2.296m ~ 0.700
ProcessOwnBlockSubtreeNodesParallel/10k_nodes-4 8.132m 8.276m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/100k_nodes-4 13.13m 13.54m ~ 0.200
ProcessOwnBlockSubtreeNodesSequential/1k_nodes-4 1.980m 1.994m ~ 0.400
ProcessOwnBlockSubtreeNodesSequential/10k_nodes-4 8.181m 7.744m ~ 0.200
ProcessOwnBlockSubtreeNodesSequential/100k_nodes-4 43.05m 40.99m ~ 0.400
DiskTxMap_SetIfNotExists-4 3.417µ 3.308µ ~ 0.700
DiskTxMap_SetIfNotExists_Parallel-4 3.108µ 3.151µ ~ 0.400
DiskTxMap_ExistenceOnly-4 287.6n 284.6n ~ 0.100
Queue-4 185.8n 185.4n ~ 1.000
AtomicPointer-4 4.470n 4.812n ~ 0.200
ReorgOptimizations/DedupFilterPipeline/Old/10K-4 837.9µ 818.4µ ~ 0.100
ReorgOptimizations/DedupFilterPipeline/New/10K-4 785.9µ 775.5µ ~ 0.100
ReorgOptimizations/AllMarkFalse/Old/10K-4 101.0µ 100.4µ ~ 0.400
ReorgOptimizations/AllMarkFalse/New/10K-4 61.95µ 61.51µ ~ 0.200
ReorgOptimizations/HashSlicePool/Old/10K-4 52.60µ 56.17µ ~ 0.700
ReorgOptimizations/HashSlicePool/New/10K-4 11.92µ 11.76µ ~ 0.700
ReorgOptimizations/NodeFlags/Old/10K-4 4.557µ 5.435µ ~ 0.100
ReorgOptimizations/NodeFlags/New/10K-4 1.506µ 2.465µ ~ 0.100
ReorgOptimizations/DedupFilterPipeline/Old/100K-4 9.140m 9.231m ~ 0.700
ReorgOptimizations/DedupFilterPipeline/New/100K-4 9.201m 9.244m ~ 0.700
ReorgOptimizations/AllMarkFalse/Old/100K-4 1.039m 1.027m ~ 0.100
ReorgOptimizations/AllMarkFalse/New/100K-4 679.3µ 678.6µ ~ 0.700
ReorgOptimizations/HashSlicePool/Old/100K-4 541.1µ 553.0µ ~ 0.400
ReorgOptimizations/HashSlicePool/New/100K-4 313.2µ 283.3µ ~ 0.200
ReorgOptimizations/NodeFlags/Old/100K-4 49.74µ 46.33µ ~ 0.100
ReorgOptimizations/NodeFlags/New/100K-4 17.26µ 16.02µ ~ 0.100
TxMapSetIfNotExists-4 52.31n 53.03n ~ 1.000
TxMapSetIfNotExistsDuplicate-4 39.73n 39.85n ~ 0.700
ChannelSendReceive-4 595.7n 601.2n ~ 0.400
BlockAssembler_AddTx-4 0.02811n 0.02840n ~ 0.400
AddNode-4 12.60 12.56 ~ 0.400
AddNodeWithMap-4 13.48 13.31 ~ 0.200
CalcBlockWork-4 529.9n 524.3n ~ 0.700
CalculateWork-4 718.3n 707.8n ~ 0.100
BuildBlockLocatorString_Helpers/Size_10-4 1.327µ 1.366µ ~ 0.100
BuildBlockLocatorString_Helpers/Size_100-4 15.81µ 14.68µ ~ 1.000
BuildBlockLocatorString_Helpers/Size_1000-4 125.7µ 129.5µ ~ 0.100
CatchupWithHeaderCache-4 104.7m 104.7m ~ 1.000
_prepareTxsPerLevel-4 417.6m 413.3m ~ 0.400
_prepareTxsPerLevelOrdered-4 3.876m 3.823m ~ 0.400
_prepareTxsPerLevel_Comparison/Original-4 429.4m 416.3m ~ 0.200
_prepareTxsPerLevel_Comparison/Optimized-4 3.727m 3.487m ~ 0.100
SubtreeSizes/10k_tx_4_per_subtree-4 1.351m 1.340m ~ 0.700
SubtreeSizes/10k_tx_16_per_subtree-4 315.1µ 320.4µ ~ 1.000
SubtreeSizes/10k_tx_64_per_subtree-4 76.40µ 75.52µ ~ 1.000
SubtreeSizes/10k_tx_256_per_subtree-4 19.09µ 18.76µ ~ 0.200
SubtreeSizes/10k_tx_512_per_subtree-4 9.386µ 9.279µ ~ 0.400
SubtreeSizes/10k_tx_1024_per_subtree-4 4.750µ 4.675µ ~ 0.400
SubtreeSizes/10k_tx_2k_per_subtree-4 2.317µ 2.329µ ~ 1.000
BlockSizeScaling/10k_tx_64_per_subtree-4 73.67µ 73.95µ ~ 0.400
BlockSizeScaling/10k_tx_256_per_subtree-4 18.64µ 18.74µ ~ 0.700
BlockSizeScaling/10k_tx_1024_per_subtree-4 4.614µ 4.635µ ~ 0.200
BlockSizeScaling/50k_tx_64_per_subtree-4 389.5µ 389.4µ ~ 1.000
BlockSizeScaling/50k_tx_256_per_subtree-4 93.03µ 93.12µ ~ 1.000
BlockSizeScaling/50k_tx_1024_per_subtree-4 22.82µ 23.15µ ~ 0.400
SubtreeAllocations/small_subtrees_exists_check-4 162.0µ 159.6µ ~ 0.700
SubtreeAllocations/small_subtrees_data_fetch-4 160.3µ 162.5µ ~ 0.400
SubtreeAllocations/small_subtrees_full_validation-4 320.9µ 321.8µ ~ 1.000
SubtreeAllocations/medium_subtrees_exists_check-4 9.512µ 9.437µ ~ 0.700
SubtreeAllocations/medium_subtrees_data_fetch-4 9.282µ 9.337µ ~ 0.700
SubtreeAllocations/medium_subtrees_full_validation-4 18.65µ 18.65µ ~ 1.000
SubtreeAllocations/large_subtrees_exists_check-4 2.266µ 2.264µ ~ 0.800
SubtreeAllocations/large_subtrees_data_fetch-4 2.257µ 2.284µ ~ 0.400
SubtreeAllocations/large_subtrees_full_validation-4 4.667µ 4.676µ ~ 0.700
_BufferPoolAllocation/16KB-4 4.609µ 5.103µ ~ 1.000
_BufferPoolAllocation/32KB-4 8.776µ 7.687µ ~ 0.700
_BufferPoolAllocation/64KB-4 17.21µ 17.39µ ~ 0.700
_BufferPoolAllocation/128KB-4 36.45µ 31.19µ ~ 0.100
_BufferPoolAllocation/512KB-4 123.9µ 114.6µ ~ 0.700
_BufferPoolConcurrent/32KB-4 19.47µ 18.79µ ~ 1.000
_BufferPoolConcurrent/64KB-4 30.83µ 30.94µ ~ 1.000
_BufferPoolConcurrent/512KB-4 153.9µ 159.8µ ~ 0.700
_SubtreeDeserializationWithBufferSizes/16KB-4 641.7µ 688.7µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/32KB-4 658.4µ 677.5µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/64KB-4 638.1µ 619.8µ ~ 1.000
_SubtreeDeserializationWithBufferSizes/128KB-4 610.9µ 641.2µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/512KB-4 601.5µ 624.0µ ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/16KB-4 36.95m 37.22m ~ 0.700
_SubtreeDataDeserializationWithBufferSizes/32KB-4 37.28m 37.05m ~ 0.200
_SubtreeDataDeserializationWithBufferSizes/64KB-4 37.17m 37.21m ~ 1.000
_SubtreeDataDeserializationWithBufferSizes/128KB-4 37.13m 36.86m ~ 0.700
_SubtreeDataDeserializationWithBufferSizes/512KB-4 36.84m 36.50m ~ 0.700
_PooledVsNonPooled/Pooled-4 833.9n 839.7n ~ 0.100
_PooledVsNonPooled/NonPooled-4 7.327µ 7.910µ ~ 0.100
_MemoryFootprint/Current_512KB_32concurrent-4 8.218µ 7.949µ ~ 0.100
_MemoryFootprint/Proposed_32KB_32concurrent-4 10.28µ 12.73µ ~ 0.100
_MemoryFootprint/Alternative_64KB_32concurrent-4 10.11µ 12.29µ ~ 0.100
StoreBlock_Sequential/BelowCSVHeight-4 334.1µ 342.0µ ~ 0.700
StoreBlock_Sequential/AboveCSVHeight-4 342.0µ 335.6µ ~ 0.100
GetUtxoHashes-4 252.1n 253.2n ~ 1.000
GetUtxoHashes_ManyOutputs-4 49.07µ 49.24µ ~ 0.700
_NewMetaDataFromBytes-4 225.1n 227.6n ~ 0.100
_Bytes-4 416.4n 415.0n ~ 1.000
_MetaBytes-4 137.4n 137.8n ~ 0.600

Threshold: >10% with p < 0.05 | Generated: 2026-05-22 13:38 UTC

@ordishs
Merge remote-tracking branch 'upstream/main' into security/4576-opinv…
ba70949 
…ert-immutable

# Conflicts:
#	services/legacy/txscript/opcode_test.go
@sonarqubecloud

sonarqubecloud Bot commented May 22, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@ordishs ordishs requested a review from sugh01 May 22, 2026 13:43
@ordishs ordishs merged commit 958f990 into bsv-blockchain:main May 26, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oskarszoon oskarszoon oskarszoon approved these changes

@sugh01 sugh01 sugh01 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ordishs @oskarszoon @sugh01