Return early in IsBanned.

gmaxwell · gmaxwell · commit bf376eaccc89 · 2017-06-09T02:05:53.000Z
I am not aware of any reason that we'd try to stop a ban-list timing
 side-channel and the prior code wouldn't be enough if we were.
diff --git a/src/net.cpp b/src/net.cpp
@@ -469,35 +469,31 @@ void CConnman::ClearBanned()
 
 bool CConnman::IsBanned(CNetAddr ip)
 {
-    bool fResult = false;
+    LOCK(cs_setBanned);
+    for (banmap_t::iterator it = setBanned.begin(); it != setBanned.end(); it++)
     {
-        LOCK(cs_setBanned);
-        for (banmap_t::iterator it = setBanned.begin(); it != setBanned.end(); it++)
-        {
-            CSubNet subNet = (*it).first;
-            CBanEntry banEntry = (*it).second;
+        CSubNet subNet = (*it).first;
+        CBanEntry banEntry = (*it).second;
 
-            if(subNet.Match(ip) && GetTime() < banEntry.nBanUntil)
-                fResult = true;
+        if (subNet.Match(ip) && GetTime() < banEntry.nBanUntil) {
+            return true;
         }
     }
-    return fResult;
+    return false;
 }
 
 bool CConnman::IsBanned(CSubNet subnet)
 {
-    bool fResult = false;
+    LOCK(cs_setBanned);
+    banmap_t::iterator i = setBanned.find(subnet);
+    if (i != setBanned.end())
     {
-        LOCK(cs_setBanned);
-        banmap_t::iterator i = setBanned.find(subnet);
-        if (i != setBanned.end())
-        {
-            CBanEntry banEntry = (*i).second;
-            if (GetTime() < banEntry.nBanUntil)
-                fResult = true;
+        CBanEntry banEntry = (*i).second;
+        if (GetTime() < banEntry.nBanUntil) {
+            return true;
         }
     }
-    return fResult;
+    return false;
 }
 
 void CConnman::Ban(const CNetAddr& addr, const BanReason &banReason, int64_t bantimeoffset, bool sinceUnixEpoch) {

Original file line number	Diff line number	Diff line change
`@@ -469,35 +469,31 @@ void CConnman::ClearBanned()`
`469`	`469`
`470`	`470`	`bool CConnman::IsBanned(CNetAddr ip)`
`471`	`471`	`{`
`472`		`- bool fResult = false;`
	`472`	`+ LOCK(cs_setBanned);`
	`473`	`+ for (banmap_t::iterator it = setBanned.begin(); it != setBanned.end(); it++)`
`473`	`474`	`{`
`474`		`- LOCK(cs_setBanned);`
`475`		`- for (banmap_t::iterator it = setBanned.begin(); it != setBanned.end(); it++)`
`476`		`- {`
`477`		`- CSubNet subNet = (*it).first;`
`478`		`- CBanEntry banEntry = (*it).second;`
	`475`	`+ CSubNet subNet = (*it).first;`
	`476`	`+ CBanEntry banEntry = (*it).second;`
`479`	`477`
`480`		`- if(subNet.Match(ip) && GetTime() < banEntry.nBanUntil)`
`481`		`- fResult = true;`
	`478`	`+ if (subNet.Match(ip) && GetTime() < banEntry.nBanUntil) {`
	`479`	`+ return true;`
`482`	`480`	`}`
`483`	`481`	`}`
`484`		`- return fResult;`
	`482`	`+ return false;`
`485`	`483`	`}`
`486`	`484`
`487`	`485`	`bool CConnman::IsBanned(CSubNet subnet)`
`488`	`486`	`{`
`489`		`- bool fResult = false;`
	`487`	`+ LOCK(cs_setBanned);`
	`488`	`+ banmap_t::iterator i = setBanned.find(subnet);`
	`489`	`+ if (i != setBanned.end())`
`490`	`490`	`{`
`491`		`- LOCK(cs_setBanned);`
`492`		`- banmap_t::iterator i = setBanned.find(subnet);`
`493`		`- if (i != setBanned.end())`
`494`		`- {`
`495`		`- CBanEntry banEntry = (*i).second;`
`496`		`- if (GetTime() < banEntry.nBanUntil)`
`497`		`- fResult = true;`
	`491`	`+ CBanEntry banEntry = (*i).second;`
	`492`	`+ if (GetTime() < banEntry.nBanUntil) {`
	`493`	`+ return true;`
`498`	`494`	`}`
`499`	`495`	`}`
`500`		`- return fResult;`
	`496`	`+ return false;`
`501`	`497`	`}`
`502`	`498`
`503`	`499`	`void CConnman::Ban(const CNetAddr& addr, const BanReason &banReason, int64_t bantimeoffset, bool sinceUnixEpoch) {`