Return early in IsBanned.

gmaxwell · furszy · commit a0a079fc778f · 2021-05-13T23:55:21.000-03:00
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
@@ -453,34 +453,30 @@ 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()) {
-            CBanEntry banEntry = (*i).second;
-            if (GetTime() < banEntry.nBanUntil)
-                fResult = true;
+    LOCK(cs_setBanned);
+    banmap_t::iterator i = setBanned.find(subnet);
+    if (i != setBanned.end()) {
+        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
`@@ -453,34 +453,30 @@ void CConnman::ClearBanned()`
`453`	`453`
`454`	`454`	`bool CConnman::IsBanned(CNetAddr ip)`
`455`	`455`	`{`
`456`		`- bool fResult = false;`
	`456`	`+ LOCK(cs_setBanned);`
	`457`	`+ for (banmap_t::iterator it = setBanned.begin(); it != setBanned.end(); it++)`
`457`	`458`	`{`
`458`		`- LOCK(cs_setBanned);`
`459`		`- for (banmap_t::iterator it = setBanned.begin(); it != setBanned.end(); it++)`
`460`		`- {`
`461`		`- CSubNet subNet = (*it).first;`
`462`		`- CBanEntry banEntry = (*it).second;`
	`459`	`+ CSubNet subNet = (*it).first;`
	`460`	`+ CBanEntry banEntry = (*it).second;`
`463`	`461`
`464`		`- if(subNet.Match(ip) && GetTime() < banEntry.nBanUntil)`
`465`		`- fResult = true;`
	`462`	`+ if (subNet.Match(ip) && GetTime() < banEntry.nBanUntil) {`
	`463`	`+ return true;`
`466`	`464`	`}`
`467`	`465`	`}`
`468`		`- return fResult;`
	`466`	`+ return false;`
`469`	`467`	`}`
`470`	`468`
`471`	`469`	`bool CConnman::IsBanned(CSubNet subnet)`
`472`	`470`	`{`
`473`		`- bool fResult = false;`
`474`		`- {`
`475`		`- LOCK(cs_setBanned);`
`476`		`- banmap_t::iterator i = setBanned.find(subnet);`
`477`		`- if (i != setBanned.end()) {`
`478`		`- CBanEntry banEntry = (*i).second;`
`479`		`- if (GetTime() < banEntry.nBanUntil)`
`480`		`- fResult = true;`
	`471`	`+ LOCK(cs_setBanned);`
	`472`	`+ banmap_t::iterator i = setBanned.find(subnet);`
	`473`	`+ if (i != setBanned.end()) {`
	`474`	`+ CBanEntry banEntry = (*i).second;`
	`475`	`+ if (GetTime() < banEntry.nBanUntil) {`
	`476`	`+ return true;`
`481`	`477`	`}`
`482`	`478`	`}`
`483`		`- return fResult;`
	`479`	`+ return false;`
`484`	`480`	`}`
`485`	`481`
`486`	`482`	`void CConnman::Ban(const CNetAddr& addr, const BanReason &banReason, int64_t bantimeoffset, bool sinceUnixEpoch)`