Skip to content

Commit 930ce34

Browse files
authored
Merge pull request #6998 from ethereum/fixSignChop
Fix sign chop
2 parents 9252906 + 04fe3c0 commit 930ce34

File tree

5 files changed

+70
-2
lines changed

5 files changed

+70
-2
lines changed

Changelog.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
Important Bugfixes:
44
* Fix incorrect abi encoding of storage array of data type that occupy multiple storage slots
5+
* Properly zero out higher order bits in elements of an array of negative numbers when assigning to storage and converting the type at the same time.
56

67

78
Compiler Features:

docs/bugs.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
11
[
2+
{
3+
"name": "SignedArrayStorageCopy",
4+
"summary": "Assigning an array of signed integers to a storage array of different type can lead to data corruption in that array.",
5+
"description": "In two's complement, negative integers have their higher order bits set. In order to fit into a shared storage slot, these have to be set to zero. When a conversion is done at the same time, the bits to set to zero were incorrectly determined from the source and not the target type. This means that such copy operations can lead to incorrect values being stored.",
6+
"introduced": "0.4.7",
7+
"fixed": "0.5.10",
8+
"severity": "low/medium"
9+
},
210
{
311
"name": "ABIEncoderV2StorageArrayWithMultiSlotElement",
412
"summary": "Storage arrays containing structs or other statically-sized arrays are not read properly when directly encoded in external function calls or in abi.encode*.",

docs/bugs_by_version.json

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,7 @@
380380
},
381381
"0.4.10": {
382382
"bugs": [
383+
"SignedArrayStorageCopy",
383384
"UninitializedFunctionPointerInConstructor_0.4.x",
384385
"IncorrectEventSignatureInLibraries_0.4.x",
385386
"ExpExponentCleanup",
@@ -394,6 +395,7 @@
394395
},
395396
"0.4.11": {
396397
"bugs": [
398+
"SignedArrayStorageCopy",
397399
"UninitializedFunctionPointerInConstructor_0.4.x",
398400
"IncorrectEventSignatureInLibraries_0.4.x",
399401
"ExpExponentCleanup",
@@ -407,6 +409,7 @@
407409
},
408410
"0.4.12": {
409411
"bugs": [
412+
"SignedArrayStorageCopy",
410413
"UninitializedFunctionPointerInConstructor_0.4.x",
411414
"IncorrectEventSignatureInLibraries_0.4.x",
412415
"ExpExponentCleanup",
@@ -419,6 +422,7 @@
419422
},
420423
"0.4.13": {
421424
"bugs": [
425+
"SignedArrayStorageCopy",
422426
"UninitializedFunctionPointerInConstructor_0.4.x",
423427
"IncorrectEventSignatureInLibraries_0.4.x",
424428
"ExpExponentCleanup",
@@ -431,6 +435,7 @@
431435
},
432436
"0.4.14": {
433437
"bugs": [
438+
"SignedArrayStorageCopy",
434439
"UninitializedFunctionPointerInConstructor_0.4.x",
435440
"IncorrectEventSignatureInLibraries_0.4.x",
436441
"ExpExponentCleanup",
@@ -442,6 +447,7 @@
442447
},
443448
"0.4.15": {
444449
"bugs": [
450+
"SignedArrayStorageCopy",
445451
"UninitializedFunctionPointerInConstructor_0.4.x",
446452
"IncorrectEventSignatureInLibraries_0.4.x",
447453
"ExpExponentCleanup",
@@ -452,6 +458,7 @@
452458
},
453459
"0.4.16": {
454460
"bugs": [
461+
"SignedArrayStorageCopy",
455462
"ABIEncoderV2StorageArrayWithMultiSlotElement",
456463
"DynamicConstructorArgumentsClippedABIV2",
457464
"UninitializedFunctionPointerInConstructor_0.4.x",
@@ -464,6 +471,7 @@
464471
},
465472
"0.4.17": {
466473
"bugs": [
474+
"SignedArrayStorageCopy",
467475
"ABIEncoderV2StorageArrayWithMultiSlotElement",
468476
"DynamicConstructorArgumentsClippedABIV2",
469477
"UninitializedFunctionPointerInConstructor_0.4.x",
@@ -477,6 +485,7 @@
477485
},
478486
"0.4.18": {
479487
"bugs": [
488+
"SignedArrayStorageCopy",
480489
"ABIEncoderV2StorageArrayWithMultiSlotElement",
481490
"DynamicConstructorArgumentsClippedABIV2",
482491
"UninitializedFunctionPointerInConstructor_0.4.x",
@@ -489,6 +498,7 @@
489498
},
490499
"0.4.19": {
491500
"bugs": [
501+
"SignedArrayStorageCopy",
492502
"ABIEncoderV2StorageArrayWithMultiSlotElement",
493503
"DynamicConstructorArgumentsClippedABIV2",
494504
"UninitializedFunctionPointerInConstructor_0.4.x",
@@ -518,6 +528,7 @@
518528
},
519529
"0.4.20": {
520530
"bugs": [
531+
"SignedArrayStorageCopy",
521532
"ABIEncoderV2StorageArrayWithMultiSlotElement",
522533
"DynamicConstructorArgumentsClippedABIV2",
523534
"UninitializedFunctionPointerInConstructor_0.4.x",
@@ -531,6 +542,7 @@
531542
},
532543
"0.4.21": {
533544
"bugs": [
545+
"SignedArrayStorageCopy",
534546
"ABIEncoderV2StorageArrayWithMultiSlotElement",
535547
"DynamicConstructorArgumentsClippedABIV2",
536548
"UninitializedFunctionPointerInConstructor_0.4.x",
@@ -544,6 +556,7 @@
544556
},
545557
"0.4.22": {
546558
"bugs": [
559+
"SignedArrayStorageCopy",
547560
"ABIEncoderV2StorageArrayWithMultiSlotElement",
548561
"DynamicConstructorArgumentsClippedABIV2",
549562
"UninitializedFunctionPointerInConstructor_0.4.x",
@@ -557,6 +570,7 @@
557570
},
558571
"0.4.23": {
559572
"bugs": [
573+
"SignedArrayStorageCopy",
560574
"ABIEncoderV2StorageArrayWithMultiSlotElement",
561575
"DynamicConstructorArgumentsClippedABIV2",
562576
"UninitializedFunctionPointerInConstructor_0.4.x",
@@ -569,6 +583,7 @@
569583
},
570584
"0.4.24": {
571585
"bugs": [
586+
"SignedArrayStorageCopy",
572587
"ABIEncoderV2StorageArrayWithMultiSlotElement",
573588
"DynamicConstructorArgumentsClippedABIV2",
574589
"UninitializedFunctionPointerInConstructor_0.4.x",
@@ -581,6 +596,7 @@
581596
},
582597
"0.4.25": {
583598
"bugs": [
599+
"SignedArrayStorageCopy",
584600
"ABIEncoderV2StorageArrayWithMultiSlotElement",
585601
"DynamicConstructorArgumentsClippedABIV2",
586602
"UninitializedFunctionPointerInConstructor_0.4.x",
@@ -591,6 +607,7 @@
591607
},
592608
"0.4.26": {
593609
"bugs": [
610+
"SignedArrayStorageCopy",
594611
"ABIEncoderV2StorageArrayWithMultiSlotElement",
595612
"DynamicConstructorArgumentsClippedABIV2"
596613
],
@@ -658,6 +675,7 @@
658675
},
659676
"0.4.7": {
660677
"bugs": [
678+
"SignedArrayStorageCopy",
661679
"UninitializedFunctionPointerInConstructor_0.4.x",
662680
"IncorrectEventSignatureInLibraries_0.4.x",
663681
"ExpExponentCleanup",
@@ -672,6 +690,7 @@
672690
},
673691
"0.4.8": {
674692
"bugs": [
693+
"SignedArrayStorageCopy",
675694
"UninitializedFunctionPointerInConstructor_0.4.x",
676695
"IncorrectEventSignatureInLibraries_0.4.x",
677696
"ExpExponentCleanup",
@@ -686,6 +705,7 @@
686705
},
687706
"0.4.9": {
688707
"bugs": [
708+
"SignedArrayStorageCopy",
689709
"UninitializedFunctionPointerInConstructor_0.4.x",
690710
"IncorrectEventSignatureInLibraries_0.4.x",
691711
"ExpExponentCleanup",
@@ -700,6 +720,7 @@
700720
},
701721
"0.5.0": {
702722
"bugs": [
723+
"SignedArrayStorageCopy",
703724
"ABIEncoderV2StorageArrayWithMultiSlotElement",
704725
"DynamicConstructorArgumentsClippedABIV2",
705726
"UninitializedFunctionPointerInConstructor",
@@ -710,6 +731,7 @@
710731
},
711732
"0.5.1": {
712733
"bugs": [
734+
"SignedArrayStorageCopy",
713735
"ABIEncoderV2StorageArrayWithMultiSlotElement",
714736
"DynamicConstructorArgumentsClippedABIV2",
715737
"UninitializedFunctionPointerInConstructor",
@@ -720,6 +742,7 @@
720742
},
721743
"0.5.2": {
722744
"bugs": [
745+
"SignedArrayStorageCopy",
723746
"ABIEncoderV2StorageArrayWithMultiSlotElement",
724747
"DynamicConstructorArgumentsClippedABIV2",
725748
"UninitializedFunctionPointerInConstructor",
@@ -730,6 +753,7 @@
730753
},
731754
"0.5.3": {
732755
"bugs": [
756+
"SignedArrayStorageCopy",
733757
"ABIEncoderV2StorageArrayWithMultiSlotElement",
734758
"DynamicConstructorArgumentsClippedABIV2",
735759
"UninitializedFunctionPointerInConstructor",
@@ -740,6 +764,7 @@
740764
},
741765
"0.5.4": {
742766
"bugs": [
767+
"SignedArrayStorageCopy",
743768
"ABIEncoderV2StorageArrayWithMultiSlotElement",
744769
"DynamicConstructorArgumentsClippedABIV2",
745770
"UninitializedFunctionPointerInConstructor",
@@ -750,6 +775,7 @@
750775
},
751776
"0.5.5": {
752777
"bugs": [
778+
"SignedArrayStorageCopy",
753779
"ABIEncoderV2StorageArrayWithMultiSlotElement",
754780
"DynamicConstructorArgumentsClippedABIV2",
755781
"UninitializedFunctionPointerInConstructor",
@@ -762,6 +788,7 @@
762788
},
763789
"0.5.6": {
764790
"bugs": [
791+
"SignedArrayStorageCopy",
765792
"ABIEncoderV2StorageArrayWithMultiSlotElement",
766793
"DynamicConstructorArgumentsClippedABIV2",
767794
"UninitializedFunctionPointerInConstructor",
@@ -773,6 +800,7 @@
773800
},
774801
"0.5.7": {
775802
"bugs": [
803+
"SignedArrayStorageCopy",
776804
"ABIEncoderV2StorageArrayWithMultiSlotElement",
777805
"DynamicConstructorArgumentsClippedABIV2",
778806
"UninitializedFunctionPointerInConstructor",
@@ -782,13 +810,15 @@
782810
},
783811
"0.5.8": {
784812
"bugs": [
813+
"SignedArrayStorageCopy",
785814
"ABIEncoderV2StorageArrayWithMultiSlotElement",
786815
"DynamicConstructorArgumentsClippedABIV2"
787816
],
788817
"released": "2019-04-30"
789818
},
790819
"0.5.9": {
791820
"bugs": [
821+
"SignedArrayStorageCopy",
792822
"ABIEncoderV2StorageArrayWithMultiSlotElement"
793823
],
794824
"released": "2019-05-28"

libsolidity/codegen/CompilerUtils.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -846,9 +846,9 @@ void CompilerUtils::convertType(
846846
cleanHigherOrderBits(targetType);
847847
if (chopSignBitsPending)
848848
{
849-
if (typeOnStack.numBits() < 256)
849+
if (targetType.numBits() < 256)
850850
m_context
851-
<< ((u256(1) << typeOnStack.numBits()) - 1)
851+
<< ((u256(1) << targetType.numBits()) - 1)
852852
<< Instruction::AND;
853853
chopSignBitsPending = false;
854854
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
contract Test {
2+
int16[] public x = [-1, -2];
3+
int16[2] public y = [-5, -6];
4+
int16 z;
5+
function f() public returns (int16[] memory) {
6+
int8[] memory t = new int8[](2);
7+
t[0] = -3;
8+
t[1] = -4;
9+
x = t;
10+
return x;
11+
}
12+
function g() public returns (int16[2] memory) {
13+
int8[2] memory t = [-3, -4];
14+
y = t;
15+
return y;
16+
}
17+
function h(int8 t) public returns (int16) {
18+
z = t;
19+
return z;
20+
}
21+
}
22+
// ----
23+
// x(uint256): 0 -> -1
24+
// x(uint256): 1 -> -2
25+
// y(uint256): 0 -> -5
26+
// y(uint256): 1 -> -6
27+
// f() -> 0x20, 2, -3, -4
28+
// g() -> -3, -4
29+
// h(int8): -10 -> -10

0 commit comments

Comments
 (0)