From d62836a9723f3a93e2e450ef474525e8b731acf4 Mon Sep 17 00:00:00 2001
From: Edd Inglis <edd.inglis@gmail.com>
Date: Mon, 31 Jan 2022 22:12:17 +0000
Subject: [PATCH 1/4] Permit using the Updater _hash function, even if we don't
 have a signature appended to the image

---
 cores/esp8266/Updater.cpp | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/cores/esp8266/Updater.cpp b/cores/esp8266/Updater.cpp
index 19a9dad528..365e0a4644 100644
--- a/cores/esp8266/Updater.cpp
+++ b/cores/esp8266/Updater.cpp
@@ -224,13 +224,17 @@ bool UpdaterClass::end(bool evenIfRemaining){
     _size = progress();
   }
 
-  uint32_t sigLen = 0;
   if (_verify) {
-    ESP.flashRead(_startAddress + _size - sizeof(uint32_t), &sigLen, sizeof(uint32_t));
+    const uint32_t expectedSigLen = _verify->length();
+    uint32_t sigLen = 0;
+
+    if (expectedSigLen > 0) {
+      ESP.flashRead(_startAddress + _size - sizeof(uint32_t), &sigLen, sizeof(uint32_t));
+    }
 #ifdef DEBUG_UPDATER
     DEBUG_UPDATER.printf_P(PSTR("[Updater] sigLen: %d\n"), sigLen);
 #endif
-    if (sigLen != _verify->length()) {
+    if (sigLen != expectedSigLen) {
       _setError(UPDATE_ERROR_SIGN);
       _reset();
       return false;
@@ -255,20 +259,24 @@ bool UpdaterClass::end(bool evenIfRemaining){
     for (int i=0; i<_hash->len(); i++) DEBUG_UPDATER.printf(" %02x", ret[i]);
     DEBUG_UPDATER.printf("\n");
 #endif
-    uint8_t *sig = (uint8_t*)malloc(sigLen);
-    if (!sig) {
-      _setError(UPDATE_ERROR_SIGN);
-      _reset();
-      return false;
-    }
-    ESP.flashRead(_startAddress + binSize, sig, sigLen);
+
+    uint8_t *sig = 0; // Safe to free if we don't actually malloc
+    if (expectedSigLen > 0) {
+      sig = (uint8_t*)malloc(sigLen);
+      if (!sig) {
+        _setError(UPDATE_ERROR_SIGN);
+        _reset();
+        return false;
+      }
+      ESP.flashRead(_startAddress + binSize, sig, sigLen);
 #ifdef DEBUG_UPDATER
-    DEBUG_UPDATER.printf_P(PSTR("[Updater] Received Signature:"));
-    for (size_t i=0; i<sigLen; i++) {
-      DEBUG_UPDATER.printf(" %02x", sig[i]);
-    }
-    DEBUG_UPDATER.printf("\n");
+      DEBUG_UPDATER.printf_P(PSTR("[Updater] Received Signature:"));
+      for (size_t i=0; i<sigLen; i++) {
+        DEBUG_UPDATER.printf(" %02x", sig[i]);
+      }
+      DEBUG_UPDATER.printf("\n");
 #endif
+    }
     if (!_verify->verify(_hash, (void *)sig, sigLen)) {
       free(sig);
       _setError(UPDATE_ERROR_SIGN);

From b716f815c0f11b3cc863b0998f6ef88a382b7395 Mon Sep 17 00:00:00 2001
From: Edd Inglis <edd.inglis@gmail.com>
Date: Mon, 31 Jan 2022 23:23:08 +0000
Subject: [PATCH 2/4] Actioning review comment to use 'nullptr' instead of '0'

---
 cores/esp8266/Updater.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cores/esp8266/Updater.cpp b/cores/esp8266/Updater.cpp
index 365e0a4644..50c388d6b8 100644
--- a/cores/esp8266/Updater.cpp
+++ b/cores/esp8266/Updater.cpp
@@ -260,7 +260,7 @@ bool UpdaterClass::end(bool evenIfRemaining){
     DEBUG_UPDATER.printf("\n");
 #endif
 
-    uint8_t *sig = 0; // Safe to free if we don't actually malloc
+    uint8_t *sig = nullptr; // Safe to free if we don't actually malloc
     if (expectedSigLen > 0) {
       sig = (uint8_t*)malloc(sigLen);
       if (!sig) {

From 5d162ce6f54fd6e5d802205b71dbacf67094b444 Mon Sep 17 00:00:00 2001
From: Edd Inglis <edd.inglis@gmail.com>
Date: Wed, 2 Feb 2022 21:44:45 +0000
Subject: [PATCH 3/4] Fixed a case where the last four bytes of data were not
 passed to the hash function, even if the expected signature length was zero,
 and added some clarifying comments

---
 cores/esp8266/Updater.cpp | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/cores/esp8266/Updater.cpp b/cores/esp8266/Updater.cpp
index 50c388d6b8..460487b98e 100644
--- a/cores/esp8266/Updater.cpp
+++ b/cores/esp8266/Updater.cpp
@@ -226,6 +226,9 @@ bool UpdaterClass::end(bool evenIfRemaining){
 
   if (_verify) {
     const uint32_t expectedSigLen = _verify->length();
+      // If expectedSigLen is non-zero, we expect the last four bytes of the buffer to
+      // contain a matching length field, preceded by the bytes of the signature itself.
+      // But if expectedSigLen is zero, we expect neither a signature nor a length field;
     uint32_t sigLen = 0;
 
     if (expectedSigLen > 0) {
@@ -240,7 +243,10 @@ bool UpdaterClass::end(bool evenIfRemaining){
       return false;
     }
 
-    int binSize = _size - sigLen - sizeof(uint32_t) /* The siglen word */;
+    int binSize = _size;
+    if (expectedSigLen > 0) {
+      _size -= (sigLen + sizeof(uint32_t) /* The siglen word */);
+    }
     _hash->begin();
 #ifdef DEBUG_UPDATER
     DEBUG_UPDATER.printf_P(PSTR("[Updater] Adjusted binsize: %d\n"), binSize);

From 4b26625bc9e493b2c4be3ac1388f7b0d50c8bbe8 Mon Sep 17 00:00:00 2001
From: Edd Inglis <edd.inglis@gmail.com>
Date: Fri, 11 Mar 2022 21:41:38 +0000
Subject: [PATCH 4/4] Catch null pointer before it can be de-referenced

---
 libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.cpp b/libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.cpp
index 741c701804..df709bdd2f 100755
--- a/libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.cpp
+++ b/libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.cpp
@@ -254,6 +254,12 @@ HTTPUpdateResult ESP8266HTTPUpdate::handleUpdate(HTTPClient& http, const String&
                 }
 
                 WiFiClient * tcp = http.getStreamPtr();
+                if (!tcp) {
+                    DEBUG_HTTP_UPDATE("[httpUpdate] WiFiClient connection unexpectedly absent\n");
+                    _setLastError(HTTPC_ERROR_CONNECTION_LOST);
+                    http.end();
+                    return HTTP_UPDATE_FAILED;
+                }
 
                 if (_closeConnectionsOnUpdate) {
                     WiFiUDP::stopAll();