Using ``msg.value`` in a loop vulnerabilitiy added.

[0xSandyy]

Related Issue

• Closes: https://github.com/kadenzipfel/smart-contract-vulnerabilities/issues/72

Checklist

• [x] I have read and followed the style guide

Describe the changes you've made:

This new vulnerability listing deals with using msg.value in a loop which has various implications. All of the implications are thoroughly mentioned along with the code examples. There is also a link to real world hack that exploited this vulnerability and all the links to the sources are listed separately. This vulnerability can be listed as a separate issue without needing a separate category.

Type of change

Select the appropriate checkbox:

• [ ] Bug fix (fixing an issue with existing vulnerability data)
• [x] New feature (adding a new vulnerability or category)
• [ ] Documentation update (improving existing information)

Additional Information

I didn't explain the hack in detail but it's not necessary as the issue description provides enough information for readers to learn and research the hack on their own without lacking any context.

[0xSandyy]

This is good, but I wonder if there are other things like this that will behave in unexpected ways in a loop that could be included here. Any ideas?

Out of gas if unbounded loop, off by ones, using break instead of continue, etc. These are only I can think of right now.

But there is one particular edge case with using unchecked{++i} code blocks to optimize gas.

for(uint256 i = 0; i < 10; ){
     if(i == 3){
       continue;
     }
     unchecked {
         ++i; // never reached when i == 3, thus unlimited looping
     }
}

In the above example, unchecked block is never reached if i == 3 as continue restarts the loop with i == 3. Thus there is unlimited looping and the transaction reverts. But, this issue can be generalized to every looping where continue is used before the loop index is updated.

Let me know if I should work on any of these.

[kadenzipfel]

This is good, but I wonder if there are other things like this that will behave in unexpected ways in a loop that could be included here. Any ideas?

Out of gas if unbounded loop, off by ones, using break instead of continue, etc. These are only I can think of right now.

But there is one particular edge case with using unchecked{++i} code blocks to optimize gas.

for(uint256 i = 0; i < 10; ){
     if(i == 3){
       continue;
     }
     unchecked {
         ++i; // never reached when i == 3, thus unlimited looping
     }
}

In the above example, unchecked block is never reached if i == 3 as continue restarts the loop with i == 3. Thus there is unlimited looping and the transaction reverts. But, this issue can be generalized to every looping where continue is used before the loop index is updated.

Let me know if I should work on any of these.

These are useful but I think not enough crossover with this vulnerability. I'm thinking something that when used in a loop can behave unexpectedly, but can't really think of examples