Add control flow analyzer and test for uninitialized storage returns.

This commit is contained in:
Daniel Kirchner 2018-05-04 15:58:24 +02:00
parent 995623f0fa
commit 16e966dea0
25 changed files with 592 additions and 0 deletions

View File

@ -0,0 +1,156 @@
/*
This file is part of solidity.
solidity is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
solidity is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with solidity. If not, see <http://www.gnu.org/licenses/>.
*/
#include <libsolidity/analysis/ControlFlowAnalyzer.h>
using namespace std;
using namespace dev::solidity;
bool ControlFlowAnalyzer::analyze(ASTNode const& _astRoot)
{
_astRoot.accept(*this);
return Error::containsOnlyWarnings(m_errorReporter.errors());
}
bool ControlFlowAnalyzer::visit(FunctionDefinition const& _function)
{
auto const& functionFlow = m_cfg.functionFlow(_function);
checkUnassignedStorageReturnValues(_function, functionFlow.entry, functionFlow.exit);
return false;
}
set<VariableDeclaration const*> ControlFlowAnalyzer::variablesAssignedInNode(CFGNode const *node)
{
set<VariableDeclaration const*> result;
for (auto expression: node->block.expressions)
{
if (auto const* assignment = dynamic_cast<Assignment const*>(expression))
{
stack<Expression const*> expressions;
expressions.push(&assignment->leftHandSide());
while (!expressions.empty())
{
Expression const* expression = expressions.top();
expressions.pop();
if (auto const *tuple = dynamic_cast<TupleExpression const*>(expression))
for (auto const& component: tuple->components())
expressions.push(component.get());
else if (auto const* identifier = dynamic_cast<Identifier const*>(expression))
if (auto const* variableDeclaration = dynamic_cast<VariableDeclaration const*>(
identifier->annotation().referencedDeclaration
))
result.insert(variableDeclaration);
}
}
}
return result;
}
void ControlFlowAnalyzer::checkUnassignedStorageReturnValues(
FunctionDefinition const& _function,
CFGNode const* _functionEntry,
CFGNode const* _functionExit
) const
{
if (_function.returnParameterList()->parameters().empty())
return;
map<CFGNode const*, set<VariableDeclaration const*>> unassigned;
{
auto& unassignedAtFunctionEntry = unassigned[_functionEntry];
for (auto const& returnParameter: _function.returnParameterList()->parameters())
if (returnParameter->type()->dataStoredIn(DataLocation::Storage))
unassignedAtFunctionEntry.insert(returnParameter.get());
}
stack<CFGNode const*> nodesToTraverse;
nodesToTraverse.push(_functionEntry);
// walk all paths from entry with maximal set of unassigned return values
while (!nodesToTraverse.empty())
{
auto node = nodesToTraverse.top();
nodesToTraverse.pop();
auto& unassignedAtNode = unassigned[node];
if (node->block.returnStatement != nullptr)
if (node->block.returnStatement->expression())
unassignedAtNode.clear();
if (!unassignedAtNode.empty())
{
// kill all return values to which a value is assigned
for (auto const* variableDeclaration: variablesAssignedInNode(node))
unassignedAtNode.erase(variableDeclaration);
// kill all return values referenced in inline assembly
// a reference is enough, checking whether there actually was an assignment might be overkill
for (auto assembly: node->block.inlineAssemblyStatements)
for (auto const& ref: assembly->annotation().externalReferences)
if (auto variableDeclaration = dynamic_cast<VariableDeclaration const*>(ref.second.declaration))
unassignedAtNode.erase(variableDeclaration);
}
for (auto const& exit: node->exits)
{
auto& unassignedAtExit = unassigned[exit];
auto oldSize = unassignedAtExit.size();
unassignedAtExit.insert(unassignedAtNode.begin(), unassignedAtNode.end());
// (re)traverse an exit, if we are on a path with new unassigned return values to consider
// this will terminate, since there is only a finite number of unassigned return values
if (unassignedAtExit.size() > oldSize)
nodesToTraverse.push(exit);
}
}
if (!unassigned[_functionExit].empty())
{
vector<VariableDeclaration const*> unassignedOrdered(
unassigned[_functionExit].begin(),
unassigned[_functionExit].end()
);
sort(
unassignedOrdered.begin(),
unassignedOrdered.end(),
[](VariableDeclaration const* lhs, VariableDeclaration const* rhs) -> bool {
return lhs->id() < rhs->id();
}
);
for (auto const* returnVal: unassignedOrdered)
{
SecondarySourceLocation ssl;
for (CFGNode* lastNodeBeforeExit: _functionExit->entries)
if (unassigned[lastNodeBeforeExit].count(returnVal))
{
if (!!lastNodeBeforeExit->block.returnStatement)
ssl.append("Problematic return:", lastNodeBeforeExit->block.returnStatement->location());
else
ssl.append("Problematic end of function:", _function.location());
}
m_errorReporter.warning(
returnVal->location(),
"This variable is of storage pointer type and might be returned without assignment. "
"This can cause storage corruption. Assign the variable (potentially from itself) "
"to remove this warning.",
ssl
);
}
}
}

View File

@ -0,0 +1,52 @@
/*
This file is part of solidity.
solidity is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
solidity is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with solidity. If not, see <http://www.gnu.org/licenses/>.
*/
#pragma once
#include <libsolidity/analysis/ControlFlowGraph.h>
#include <set>
namespace dev
{
namespace solidity
{
class ControlFlowAnalyzer: private ASTConstVisitor
{
public:
explicit ControlFlowAnalyzer(CFG const& _cfg, ErrorReporter& _errorReporter):
m_cfg(_cfg), m_errorReporter(_errorReporter) {}
bool analyze(ASTNode const& _astRoot);
virtual bool visit(FunctionDefinition const& _function) override;
private:
static std::set<VariableDeclaration const*> variablesAssignedInNode(CFGNode const *node);
void checkUnassignedStorageReturnValues(
FunctionDefinition const& _function,
CFGNode const* _functionEntry,
CFGNode const* _functionExit
) const;
CFG const& m_cfg;
ErrorReporter& m_errorReporter;
};
}
}

View File

@ -29,6 +29,7 @@
#include <libsolidity/ast/AST.h>
#include <libsolidity/parsing/Scanner.h>
#include <libsolidity/parsing/Parser.h>
#include <libsolidity/analysis/ControlFlowAnalyzer.h>
#include <libsolidity/analysis/ControlFlowGraph.h>
#include <libsolidity/analysis/GlobalContext.h>
#include <libsolidity/analysis/NameAndTypeResolver.h>
@ -229,6 +230,14 @@ bool CompilerStack::analyze()
for (Source const* source: m_sourceOrder)
if (!cfg.constructFlow(*source->ast))
noErrors = false;
if (noErrors)
{
ControlFlowAnalyzer controlFlowAnalyzer(cfg, m_errorReporter);
for (Source const* source: m_sourceOrder)
if (!controlFlowAnalyzer.analyze(*source->ast))
noErrors = false;
}
}
if (noErrors)

View File

@ -0,0 +1,26 @@
contract C {
struct S { bool f; }
S s;
function f() internal returns (S storage c) {
assembly {
sstore(c_slot, sload(s_slot))
}
}
function g(bool flag) internal returns (S storage c) {
// control flow in assembly will not be analyzed for now,
// so this will not issue a warning
assembly {
if flag {
sstore(c_slot, sload(s_slot))
}
}
}
function h() internal returns (S storage c) {
// any reference from assembly will be sufficient for now,
// so this will not issue a warning
assembly {
sstore(s_slot, sload(c_slot))
}
}
}
// ----

View File

@ -0,0 +1,10 @@
contract C {
struct S { bool f; }
S s;
function f() internal pure returns (S storage) {
assembly {
}
}
}
// ----
// Warning: (87-88): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.

View File

@ -0,0 +1,36 @@
contract C {
struct S { bool f; }
S s;
function f() internal view returns (S storage c) {
do {} while((c = s).f);
}
function g() internal view returns (S storage c) {
do { c = s; } while(false);
}
function h() internal view returns (S storage c) {
c = s;
do {} while(false);
}
function i() internal view returns (S storage c) {
do {} while(false);
c = s;
}
function j() internal view returns (S storage c) {
do {
c = s;
break;
} while(false);
}
function k() internal view returns (S storage c) {
do {
if (s.f) {
continue;
break;
}
else {
c = s;
}
} while(false);
}
}
// ----

View File

@ -0,0 +1,35 @@
contract C {
struct S { bool f; }
S s;
function f() internal view returns (S storage c) {
do {
break;
c = s;
} while(false);
}
function g() internal view returns (S storage c) {
do {
if (s.f) {
continue;
c = s;
}
else {
}
} while(false);
}
function h() internal view returns (S storage c) {
do {
if (s.f) {
break;
continue;
}
else {
c = s;
}
} while(false);
}
}
// ----
// Warning: (87-98): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
// Warning: (223-234): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
// Warning: (440-451): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.

View File

@ -0,0 +1,6 @@
contract C {
struct S { bool f; }
S s;
function f() internal view returns (S storage c, S storage d) { c = s; d = s; return; }
}
// ----

View File

@ -0,0 +1,15 @@
contract C {
struct S { bool f; }
S s;
function f() internal pure returns (S storage) { return; }
function g() internal view returns (S storage c, S storage) { c = s; return; }
function h() internal view returns (S storage, S storage d) { d = s; return; }
function i() internal pure returns (S storage, S storage) { return; }
function j() internal view returns (S storage, S storage) { return (s,s); }
}
// ----
// Warning: (87-88): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
// Warning: (163-164): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
// Warning: (233-234): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
// Warning: (316-317): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
// Warning: (327-328): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.

View File

@ -0,0 +1,13 @@
contract C {
struct S { bool f; }
S s;
function f() internal view returns (S storage c) {
for(c = s;;) {
}
}
function g() internal view returns (S storage c) {
for(; (c = s).f;) {
}
}
}
// ----

View File

@ -0,0 +1,16 @@
contract C {
struct S { bool f; }
S s;
function f() internal view returns (S storage c) {
for(;; c = s) {
}
}
function g() internal view returns (S storage c) {
for(;;) {
c = s;
}
}
}
// ----
// Warning: (87-98): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
// Warning: (182-193): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.

View File

@ -0,0 +1,29 @@
contract C {
struct S { bool f; }
S s;
function f(bool flag) internal view returns (S storage c) {
if (flag) c = s;
else c = s;
}
function g(bool flag) internal view returns (S storage c) {
if (flag) c = s;
else { c = s; }
}
function h(bool flag) internal view returns (S storage c) {
if (flag) c = s;
else
{
if (!flag) c = s;
else c = s;
}
}
function i() internal view returns (S storage c) {
if ((c = s).f) {
}
}
function j() internal view returns (S storage c) {
if ((c = s).f && !(c = s).f) {
}
}
}
// ----

View File

@ -0,0 +1,18 @@
contract C {
struct S { bool f; }
S s;
function f(bool flag) internal view returns (S storage c) {
if (flag) c = s;
}
function g(bool flag) internal returns (S storage c) {
if (flag) c = s;
else
{
if (!flag) c = s;
else s.f = true;
}
}
}
// ----
// Warning: (96-107): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
// Warning: (186-197): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.

View File

@ -0,0 +1,20 @@
contract C {
modifier revertIfNoReturn() {
_;
revert();
}
modifier ifFlag(bool flag) {
if (flag)
_;
}
struct S { uint a; }
S s;
function f(bool flag) revertIfNoReturn() internal view returns(S storage) {
if (flag) return s;
}
function g(bool flag) revertIfNoReturn() ifFlag(flag) internal view returns(S storage) {
return s;
}
}
// ----

View File

@ -0,0 +1,22 @@
contract C {
modifier revertIfNoReturn() {
_;
revert();
}
modifier ifFlag(bool flag) {
if (flag)
_;
}
struct S { uint a; }
S s;
function f(bool flag) ifFlag(flag) internal view returns(S storage) {
return s;
}
function g(bool flag) ifFlag(flag) revertIfNoReturn() internal view returns(S storage) {
return s;
}
}
// ----
// Warning: (249-250): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
// Warning: (367-368): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.

View File

@ -0,0 +1,12 @@
contract C {
struct S { bool f; }
S s;
function f() internal pure returns (S storage) {
revert();
}
function g(bool flag) internal view returns (S storage c) {
if (flag) c = s;
else revert();
}
}
// ----

View File

@ -0,0 +1,11 @@
contract C {
struct S { bool f; }
S s;
function f() internal view returns (S storage c) {
(c = s).f && false;
}
function g() internal view returns (S storage c) {
(c = s).f || true;
}
}
// ----

View File

@ -0,0 +1,18 @@
contract C {
struct S { bool f; }
S s;
function f() internal view returns (S storage c) {
false && (c = s).f;
}
function g() internal view returns (S storage c) {
true || (c = s).f;
}
function h() internal view returns (S storage c) {
// expect warning, although this is always fine
true && (false || (c = s).f);
}
}
// ----
// Warning: (87-98): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
// Warning: (176-187): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
// Warning: (264-275): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.

View File

@ -0,0 +1,10 @@
contract C {
struct S { bool f; }
S s;
function f() internal pure {}
function g() internal view returns (S storage) { return s; }
function h() internal view returns (S storage c) { return s; }
function i() internal view returns (S storage c) { c = s; }
function j() internal view returns (S storage c) { (c) = s; }
}
// ----

View File

@ -0,0 +1,14 @@
contract C {
struct S { bool f; }
S s;
function f(bool flag) internal view returns (S storage c) {
flag ? c = s : c = s;
}
function g(bool flag) internal view returns (S storage c) {
flag ? c = s : (c = s);
}
function h(bool flag) internal view returns (S storage c) {
flag ? (c = s).f : (c = s).f;
}
}
// ----

View File

@ -0,0 +1,13 @@
contract C {
struct S { bool f; }
S s;
function f(bool flag) internal view returns (S storage c) {
flag ? (c = s).f : false;
}
function g(bool flag) internal view returns (S storage c) {
flag ? false : (c = s).f;
}
}
// ----
// Warning: (96-107): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.
// Warning: (200-211): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.

View File

@ -0,0 +1,9 @@
contract C {
struct S { bool f; }
S s;
function f() internal pure returns (S storage) {
throw;
}
}
// ----
// Warning: (108-113): "throw" is deprecated in favour of "revert()", "require()" and "assert()".

View File

@ -0,0 +1,12 @@
contract C {
struct S { bool f; }
S s;
function f() internal view returns (S storage, uint) {
return (s,2);
}
function g() internal view returns (S storage c) {
uint a;
(c, a) = f();
}
}
// ----

View File

@ -0,0 +1,19 @@
contract C {
struct S { bool f; }
S s;
function f() internal view returns (S storage c) {
while((c = s).f) {
}
}
function g() internal view returns (S storage c) {
c = s;
while(false) {
}
}
function h() internal view returns (S storage c) {
while(false) {
}
c = s;
}
}
// ----

View File

@ -0,0 +1,11 @@
contract C {
struct S { bool f; }
S s;
function f() internal view returns (S storage c) {
while(false) {
c = s;
}
}
}
// ----
// Warning: (87-98): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.