Instantly share code, notes, and snippets.

Embed
What would you like to do?
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

This comment has been minimized.

Show comment
Hide comment
@achow101

achow101 Jun 28, 2018

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.

Owner

achow101 commented Jun 28, 2018

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