Skip to content

Commit

Permalink
Add Solidity Test Coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
gjackson12 committed Aug 23, 2018
1 parent 1cd066b commit 10f7dc3
Show file tree
Hide file tree
Showing 22 changed files with 3,404 additions and 17 deletions.
3 changes: 3 additions & 0 deletions .solcover.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
copyPackages: ['openzeppelin-solidity']
};
24 changes: 10 additions & 14 deletions avoiding_common_mistakes.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ I took some simple steps to make sure I avoid common mistakes when it comes to t
The sections below describes the steps taken to mitigate against potential contract vulnerabilities.

## 1. Logic Bugs
Simple programming mistakes can cause the contract to behave differently to its stated rules, especially on 'edge cases'.
Simple programming mistakes can cause the contract to behave differently to its stated rules.

I have mitigated against this risk by:

* Following Solidity coding standards and general coding best practices for safety-critical software.
* Avoiding overly complex rules (even at the cost of some functionality) or complicated implementation (even at the cost of some gas).

Note that I have chosen not to include a mechanism for fixing bugs during the life of the contract due to concern that this mechanism would itself be a serious vulnerability.
* Following Solidity coding standards and general coding best practices.
* Avoiding overly complex rules.

## 2. Poison Data
Contracts that accept user input that is stored or exposed to other users are vulnerable to being supplied with unanticipated input that causes problems for the contract or for other users of the contract.
Expand All @@ -25,7 +23,7 @@ I have mitigated against this risk by:
It is easy to accidentally expose a contract function which was meant to be internal, or to omit protection on a function which was meant to be called only by priviledged accounts (e.g. by the creator). I made sure to mark every function appropriatley, and leverage tested libraries like Ownable by Open Zeppelin to control which functions can only be executed by the contract owner.

## 4. Exposed Secrets
All code and data on the blockchain is visible by anyone, even if not marked as "public" in Solidity. Contracts that attempt to rely on keeping keys or behaviour secret are in for a surpsise.
All code and data on the blockchain is visible by anyone, even if not marked as "public" in Solidity.

I have mitigated against this risk by:

Expand All @@ -37,22 +35,20 @@ An attacker may cause inconvenience for other users by supplying the contract wi
I have mitigated against this risk by:

* Limiting the length of user-supplied data such as the name, description, and tags for a media asset.
* Avoiding looping behaviour where e.g. a function costs more and more gas each time is used.

## 6. Other Vulnerabilities
Even without serious collusion, Ethereum miners have some limited ability to influence block timestamps and which transactions are chosen in a block (and hence block hashes). A miner or group of miners who control a majority of hashing power in the network can make almost any change they want to contract data or behaviour.
Even without serious collusion, Ethereum miners have some limited ability to influence block timestamps and which transactions are chosen in a block.

We have mitigated against this risk by:

* Not using block hashes.
* Not expecting a precision of better than fifteen minutes or so from block timestamps.
* Not leveraging block timestamps for critical contract logic.

## 7. Tx.Origin Problem
This is kind of a "confused depty" problem. If a contract relies on Solidity 'tx.origin' to decide who the caller is (e.g. to know who to set the poster of media is), there's a danger that a malicious intermediary contract could make calls to the contract on behalf of the user (who presumably thought the malicious intermediary contract would do something else).

We have mitigated against this risk by:
If a contract relies on Solidity 'tx.origin' to decide who the caller is (e.g. to know who to set the poster of media is), there's a danger that a malicious intermediary contract could make calls to the contract on behalf of the user (who presumably thought the malicious intermediary contract would do something else).

* Not using tx.origin.
I have mitigated against this risk by:

## 8. Solidity Function Signatures and Fallback Data
* Not using tx.origin and instead msg.sender

## 8.
1 change: 1 addition & 0 deletions coverage.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"contracts/MediaGallery.sol":{"l":{"53":7,"57":6,"62":1,"67":1,"87":6,"88":5,"89":4,"90":3,"92":2,"101":2,"103":2,"104":2,"105":2,"106":2,"107":2,"108":2,"109":2,"110":2,"111":2,"114":2,"117":2,"127":2,"137":2,"139":1,"141":1,"143":1,"144":1,"145":1,"147":1,"156":6,"157":6,"158":6,"159":1,"162":5,"171":5,"172":5,"173":5,"174":1,"177":4,"186":4,"187":4,"188":4,"189":1,"192":3,"201":3,"202":3,"203":3,"204":1,"207":2},"path":"/Users/grahamjackson/Projects/media_gallery/contracts/MediaGallery.sol","s":{"1":7,"2":1,"3":1,"4":6,"5":5,"6":4,"7":3,"8":2,"9":2,"10":2,"11":2,"12":2,"13":2,"14":2,"15":2,"16":2,"17":2,"18":2,"19":2,"20":1,"21":1,"22":1,"23":1,"24":1,"25":6,"26":6,"27":6,"28":1,"29":5,"30":5,"31":5,"32":5,"33":1,"34":4,"35":4,"36":4,"37":4,"38":1,"39":3,"40":3,"41":3,"42":3,"43":1,"44":2},"b":{"1":[6,1],"2":[5,1],"3":[4,1],"4":[3,1],"5":[2,1],"6":[1,1],"7":[1,5],"8":[1,4],"9":[1,3],"10":[1,2]},"f":{"1":7,"2":1,"3":1,"4":6,"5":2,"6":6,"7":5,"8":4,"9":3},"fnMap":{"1":{"name":"stoppedInEmergency","line":52,"loc":{"start":{"line":52,"column":4},"end":{"line":52,"column":29}}},"2":{"name":"stopContract","line":61,"loc":{"start":{"line":61,"column":4},"end":{"line":61,"column":42}}},"3":{"name":"resumeContract","line":66,"loc":{"start":{"line":66,"column":4},"end":{"line":66,"column":44}}},"4":{"name":"addMedia","line":79,"loc":{"start":{"line":79,"column":4},"end":{"line":86,"column":38}}},"5":{"name":"getMediaByAddress","line":135,"loc":{"start":{"line":135,"column":4},"end":{"line":135,"column":72}}},"6":{"name":"validateName","line":155,"loc":{"start":{"line":155,"column":4},"end":{"line":155,"column":64}}},"7":{"name":"validateDesc","line":170,"loc":{"start":{"line":170,"column":4},"end":{"line":170,"column":71}}},"8":{"name":"validateTags","line":185,"loc":{"start":{"line":185,"column":4},"end":{"line":185,"column":64}}},"9":{"name":"validateHashLength","line":200,"loc":{"start":{"line":200,"column":4},"end":{"line":200,"column":70}}}},"statementMap":{"1":{"start":{"line":53,"column":8},"end":{"line":53,"column":2693}},"2":{"start":{"line":62,"column":8},"end":{"line":62,"column":23}},"3":{"start":{"line":67,"column":8},"end":{"line":67,"column":24}},"4":{"start":{"line":87,"column":8},"end":{"line":87,"column":58}},"5":{"start":{"line":88,"column":8},"end":{"line":88,"column":72}},"6":{"start":{"line":89,"column":8},"end":{"line":89,"column":58}},"7":{"start":{"line":90,"column":8},"end":{"line":90,"column":69}},"8":{"start":{"line":101,"column":8},"end":{"line":101,"column":34}},"9":{"start":{"line":103,"column":8},"end":{"line":103,"column":37}},"10":{"start":{"line":104,"column":8},"end":{"line":104,"column":32}},"11":{"start":{"line":105,"column":8},"end":{"line":105,"column":46}},"12":{"start":{"line":106,"column":8},"end":{"line":106,"column":39}},"13":{"start":{"line":107,"column":8},"end":{"line":107,"column":37}},"14":{"start":{"line":108,"column":8},"end":{"line":108,"column":42}},"15":{"start":{"line":109,"column":8},"end":{"line":109,"column":32}},"16":{"start":{"line":110,"column":8},"end":{"line":110,"column":42}},"17":{"start":{"line":111,"column":8},"end":{"line":111,"column":42}},"18":{"start":{"line":117,"column":8},"end":{"line":117,"column":5020}},"19":{"start":{"line":137,"column":8},"end":{"line":137,"column":79}},"20":{"start":{"line":139,"column":8},"end":{"line":139,"column":76}},"21":{"start":{"line":141,"column":8},"end":{"line":141,"column":35}},"22":{"start":{"line":143,"column":8},"end":{"line":143,"column":5877}},"23":{"start":{"line":144,"column":12},"end":{"line":144,"column":74}},"24":{"start":{"line":147,"column":8},"end":{"line":147,"column":28}},"25":{"start":{"line":156,"column":8},"end":{"line":156,"column":44}},"26":{"start":{"line":157,"column":8},"end":{"line":157,"column":42}},"27":{"start":{"line":158,"column":8},"end":{"line":158,"column":6425}},"28":{"start":{"line":159,"column":12},"end":{"line":159,"column":24}},"29":{"start":{"line":162,"column":8},"end":{"line":162,"column":19}},"30":{"start":{"line":171,"column":8},"end":{"line":171,"column":51}},"31":{"start":{"line":172,"column":8},"end":{"line":172,"column":42}},"32":{"start":{"line":173,"column":8},"end":{"line":173,"column":6896}},"33":{"start":{"line":174,"column":12},"end":{"line":174,"column":24}},"34":{"start":{"line":177,"column":8},"end":{"line":177,"column":19}},"35":{"start":{"line":186,"column":8},"end":{"line":186,"column":44}},"36":{"start":{"line":187,"column":8},"end":{"line":187,"column":41}},"37":{"start":{"line":188,"column":8},"end":{"line":188,"column":7331}},"38":{"start":{"line":189,"column":12},"end":{"line":189,"column":24}},"39":{"start":{"line":192,"column":8},"end":{"line":192,"column":19}},"40":{"start":{"line":201,"column":8},"end":{"line":201,"column":44}},"41":{"start":{"line":202,"column":8},"end":{"line":202,"column":41}},"42":{"start":{"line":203,"column":8},"end":{"line":203,"column":7776}},"43":{"start":{"line":204,"column":12},"end":{"line":204,"column":24}},"44":{"start":{"line":207,"column":8},"end":{"line":207,"column":19}}},"branchMap":{"1":{"line":53,"type":"if","locations":[{"start":{"line":53,"column":8},"end":{"line":53,"column":8}},{"start":{"line":53,"column":8},"end":{"line":53,"column":8}}]},"2":{"line":87,"type":"if","locations":[{"start":{"line":87,"column":8},"end":{"line":87,"column":8}},{"start":{"line":87,"column":8},"end":{"line":87,"column":8}}]},"3":{"line":88,"type":"if","locations":[{"start":{"line":88,"column":8},"end":{"line":88,"column":8}},{"start":{"line":88,"column":8},"end":{"line":88,"column":8}}]},"4":{"line":89,"type":"if","locations":[{"start":{"line":89,"column":8},"end":{"line":89,"column":8}},{"start":{"line":89,"column":8},"end":{"line":89,"column":8}}]},"5":{"line":90,"type":"if","locations":[{"start":{"line":90,"column":8},"end":{"line":90,"column":8}},{"start":{"line":90,"column":8},"end":{"line":90,"column":8}}]},"6":{"line":137,"type":"if","locations":[{"start":{"line":137,"column":8},"end":{"line":137,"column":8}},{"start":{"line":137,"column":8},"end":{"line":137,"column":8}}]},"7":{"line":158,"type":"if","locations":[{"start":{"line":158,"column":8},"end":{"line":158,"column":8}},{"start":{"line":158,"column":8},"end":{"line":158,"column":8}}]},"8":{"line":173,"type":"if","locations":[{"start":{"line":173,"column":8},"end":{"line":173,"column":8}},{"start":{"line":173,"column":8},"end":{"line":173,"column":8}}]},"9":{"line":188,"type":"if","locations":[{"start":{"line":188,"column":8},"end":{"line":188,"column":8}},{"start":{"line":188,"column":8},"end":{"line":188,"column":8}}]},"10":{"line":203,"type":"if","locations":[{"start":{"line":203,"column":8},"end":{"line":203,"column":8}},{"start":{"line":203,"column":8},"end":{"line":203,"column":8}}]}}}}
213 changes: 213 additions & 0 deletions coverage/base.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
body, html {
margin:0; padding: 0;
height: 100%;
}
body {
font-family: Helvetica Neue, Helvetica, Arial;
font-size: 14px;
color:#333;
}
.small { font-size: 12px; }
*, *:after, *:before {
-webkit-box-sizing:border-box;
-moz-box-sizing:border-box;
box-sizing:border-box;
}
h1 { font-size: 20px; margin: 0;}
h2 { font-size: 14px; }
pre {
font: 12px/1.4 Consolas, "Liberation Mono", Menlo, Courier, monospace;
margin: 0;
padding: 0;
-moz-tab-size: 2;
-o-tab-size: 2;
tab-size: 2;
}
a { color:#0074D9; text-decoration:none; }
a:hover { text-decoration:underline; }
.strong { font-weight: bold; }
.space-top1 { padding: 10px 0 0 0; }
.pad2y { padding: 20px 0; }
.pad1y { padding: 10px 0; }
.pad2x { padding: 0 20px; }
.pad2 { padding: 20px; }
.pad1 { padding: 10px; }
.space-left2 { padding-left:55px; }
.space-right2 { padding-right:20px; }
.center { text-align:center; }
.clearfix { display:block; }
.clearfix:after {
content:'';
display:block;
height:0;
clear:both;
visibility:hidden;
}
.fl { float: left; }
@media only screen and (max-width:640px) {
.col3 { width:100%; max-width:100%; }
.hide-mobile { display:none!important; }
}

.quiet {
color: #7f7f7f;
color: rgba(0,0,0,0.5);
}
.quiet a { opacity: 0.7; }

.fraction {
font-family: Consolas, 'Liberation Mono', Menlo, Courier, monospace;
font-size: 10px;
color: #555;
background: #E8E8E8;
padding: 4px 5px;
border-radius: 3px;
vertical-align: middle;
}

div.path a:link, div.path a:visited { color: #333; }
table.coverage {
border-collapse: collapse;
margin: 10px 0 0 0;
padding: 0;
}

table.coverage td {
margin: 0;
padding: 0;
vertical-align: top;
}
table.coverage td.line-count {
text-align: right;
padding: 0 5px 0 20px;
}
table.coverage td.line-coverage {
text-align: right;
padding-right: 10px;
min-width:20px;
}

table.coverage td span.cline-any {
display: inline-block;
padding: 0 5px;
width: 100%;
}
.missing-if-branch {
display: inline-block;
margin-right: 5px;
border-radius: 3px;
position: relative;
padding: 0 4px;
background: #333;
color: yellow;
}

.skip-if-branch {
display: none;
margin-right: 10px;
position: relative;
padding: 0 4px;
background: #ccc;
color: white;
}
.missing-if-branch .typ, .skip-if-branch .typ {
color: inherit !important;
}
.coverage-summary {
border-collapse: collapse;
width: 100%;
}
.coverage-summary tr { border-bottom: 1px solid #bbb; }
.keyline-all { border: 1px solid #ddd; }
.coverage-summary td, .coverage-summary th { padding: 10px; }
.coverage-summary tbody { border: 1px solid #bbb; }
.coverage-summary td { border-right: 1px solid #bbb; }
.coverage-summary td:last-child { border-right: none; }
.coverage-summary th {
text-align: left;
font-weight: normal;
white-space: nowrap;
}
.coverage-summary th.file { border-right: none !important; }
.coverage-summary th.pct { }
.coverage-summary th.pic,
.coverage-summary th.abs,
.coverage-summary td.pct,
.coverage-summary td.abs { text-align: right; }
.coverage-summary td.file { white-space: nowrap; }
.coverage-summary td.pic { min-width: 120px !important; }
.coverage-summary tfoot td { }

.coverage-summary .sorter {
height: 10px;
width: 7px;
display: inline-block;
margin-left: 0.5em;
background: url(sort-arrow-sprite.png) no-repeat scroll 0 0 transparent;
}
.coverage-summary .sorted .sorter {
background-position: 0 -20px;
}
.coverage-summary .sorted-desc .sorter {
background-position: 0 -10px;
}
.status-line { height: 10px; }
/* dark red */
.red.solid, .status-line.low, .low .cover-fill { background:#C21F39 }
.low .chart { border:1px solid #C21F39 }
/* medium red */
.cstat-no, .fstat-no, .cbranch-no, .cbranch-no { background:#F6C6CE }
/* light red */
.low, .cline-no { background:#FCE1E5 }
/* light green */
.high, .cline-yes { background:rgb(230,245,208) }
/* medium green */
.cstat-yes { background:rgb(161,215,106) }
/* dark green */
.status-line.high, .high .cover-fill { background:rgb(77,146,33) }
.high .chart { border:1px solid rgb(77,146,33) }
/* dark yellow (gold) */
.medium .chart { border:1px solid #f9cd0b; }
.status-line.medium, .medium .cover-fill { background: #f9cd0b; }
/* light yellow */
.medium { background: #fff4c2; }
/* light gray */
span.cline-neutral { background: #eaeaea; }

.cbranch-no { background: yellow !important; color: #111; }

.cstat-skip { background: #ddd; color: #111; }
.fstat-skip { background: #ddd; color: #111 !important; }
.cbranch-skip { background: #ddd !important; color: #111; }


.cover-fill, .cover-empty {
display:inline-block;
height: 12px;
}
.chart {
line-height: 0;
}
.cover-empty {
background: white;
}
.cover-full {
border-right: none !important;
}
pre.prettyprint {
border: none !important;
padding: 0 !important;
margin: 0 !important;
}
.com { color: #999 !important; }
.ignore-none { color: #999; font-weight: normal; }

.wrapper {
min-height: 100%;
height: auto !important;
height: 100%;
margin: 0 auto -48px;
}
.footer, .push {
height: 48px;
}
Loading

0 comments on commit 10f7dc3

Please sign in to comment.