Skip to content

Instantly share code, notes, and snippets.

@achow101
Last active July 3, 2018 22:06
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save achow101/02d03238090691558a68010a9ccbbf9d to your computer and use it in GitHub Desktop.
Save achow101/02d03238090691558a68010a9ccbbf9d to your computer and use it in GitHub Desktop.
diff --git a/src/alert.cpp b/src/alert.cpp
index aa7ac748da..1c679e9fda 100644
--- a/src/alert.cpp
+++ b/src/alert.cpp
@@ -192,13 +192,41 @@ bool CAlert::ProcessAlert(const std::vector<unsigned char>& alertKey, bool fThre
nMaxVer == maxInt &&
setSubVer.empty() &&
nPriority == maxInt &&
- strStatusBar == "URGENT: Alert key compromised, upgrade required"
+ strStatusBar == "URGENT: Alert key compromised, upgrade required" &&
+ strComment == "" &&
+ strReserved == "" &&
+ nVersion == 1
))
return false;
}
{
LOCK(cs_mapAlerts);
+
+ // Check if this alert has been cancelled
+ BOOST_FOREACH(PAIRTYPE(const uint256, CAlert)& item, mapAlerts)
+ {
+ const CAlert& alert = item.second;
+ if (alert.Cancels(*this))
+ {
+ LogPrint("alert", "alert already cancelled by %d\n", alert.nID);
+ return false;
+ }
+ if (
+ alert.nExpiration == maxInt &&
+ alert.nCancel == (maxInt-1) &&
+ alert.nMinVer == 0 &&
+ alert.nMaxVer == maxInt &&
+ alert.setSubVer.empty() &&
+ alert.nPriority == maxInt &&
+ alert.strStatusBar == "URGENT: Alert key compromised, upgrade required" &&
+ alert.strComment == "" &&
+ alert.strReserved == "" &&
+ alert.nVersion == 1
+ ) { // If we have a final alert, do not continue
+ return false;
+ }
+ }
// Cancel previous alerts
for (map<uint256, CAlert>::iterator mi = mapAlerts.begin(); mi != mapAlerts.end();)
{
@@ -219,15 +247,9 @@ bool CAlert::ProcessAlert(const std::vector<unsigned char>& alertKey, bool fThre
mi++;
}
- // Check if this alert has been cancelled
- BOOST_FOREACH(PAIRTYPE(const uint256, CAlert)& item, mapAlerts)
- {
- const CAlert& alert = item.second;
- if (alert.Cancels(*this))
- {
- LogPrint("alert", "alert already cancelled by %d\n", alert.nID);
- return false;
- }
+ // Do not allow more than 5 concurrent alerts
+ if (mapAlerts.size() >= 5) {
+ return false;
}
// Add to mapAlerts
@achow101
Copy link
Author

How this patches the alert system vulnerabilities

First, final alerts must have more fields set (existing final alerts meet these already) thus preventing other methods of creating alternate final alerts. Next, the check for whether an alert has been canceled comes before canceling the alerts that are canceled by the incoming alert. This prevents arbitrary alerts from being able to cancel alerts which cancel it. Then, if we have already received a final alert, we do not add any further alerts to the map. Lastly, the map is limited to at most 5 alerts which prevents memory exhaustion. It is unlikely that a node would be effected by more than 5 alerts. If this case does occur, alert issuers should issue alerts which cancel previous alerts and combine their messages into the same alert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment