ESP32 Stack Smashing Protect error

Hi

I am getting the above error at the same point in my code each cycle. It’s at the point when there is some json telemetry being sent over mqtt.

code snip as follows:

void Gate::reportTelemetry() {
  DynamicJsonBuffer jsonBuffer;
  //settings;
  //status
  //time
  JsonObject &root = jsonBuffer.createObject();

  root["version"] = VERSION;
  root["ssid"] = WiFi.SSID();
  int rssi = WiFi.RSSI();
  int quality;
  if (rssi >= -50) {
    quality = 100;
  } else if (rssi <= -100) {
    quality = 0;
  } else {
    quality = 2 * (rssi + 100);
  }
  root["rssi"] = quality;
  root["ip"] = WiFi.localIP().toString().c_str();

  time_t n = localNow(now());
  char _t[22];
  sprintf(_t, "%02d:%02d:%02d %02d/%02d/%d", hour(n), minute(n), second(n), day(n), month(n), year(n) );
  root["time"] = _t;
  JsonObject &status = root.createNestedObject("status");
  switch (state) {
    case GATE_CLOSED:
      status["state"] = "shut";
      break;
    case GATE_OPEN:
      status["state"] = "open";
      break;
    case GATE_HELD_OPEN:
      status["state"] = "held open";
      break;
    case GATE_HELD_UNTIL:
      status["state"] = "held until";
      status["until"] = closeTime;
      break;
  }
  char suffix[13] = "report/state";
  char topic[sizeof(settings.mqttTopic) + 13];
  strcat(topic, settings.mqttTopic);
  strcat(topic, suffix);
  char j[root.measureLength() + 1];
  root.prettyPrintTo(Serial);
  root.printTo(j, sizeof(j));
  client->publish(topic, j);
  lastTelemetry = millis();
}

the terminal output shows the following:

{
17:08:55.666 ->   "version": "0.2.0",
17:08:55.666 ->   "ssid": "Gaia-Rabastens-Free",
17:08:55.666 ->   "rssi": 66,
17:08:55.666 ->   "ip": "192.168.0.44",
17:08:55.666 ->   "time": "17:08:55 30/09/2018",
17:08:55.666 ->   "status": {
17:08:55.666 ->     "state": "shut"
17:08:55.701 ->   }
17:08:55.701 -> }
17:08:55.701 -> Stack smashing protect failure!
17:08:55.701 -> 
17:08:55.701 -> abort() was called at PC 0x400e9540 on core 1
17:08:55.701 -> 
17:08:55.701 -> Backtrace: 0x4008f8f4:0x3ffb1df0 0x4008faf7:0x3ffb1e10 0x400e9540:0x3ffb1e30 0x400d49bf:0x3ffb1e50 0x400d5547:0x3ffb1f20 0x400d559a:0x3ffb1f60 0x4013333e:0x3ffb1fa0
17:08:55.701 -> 
17:08:55.701 -> Rebooting...
17

the above is the prettyPrint from the json telemetry.

unfortunately I have been unable to install the backtrace tool on my system, so have not been able to recode the backtrace data.

I am using the mqtt PubSub library from Mike Knolleary (latest version).

thanks in advance for any input.

  char topic[sizeof(settings.mqttTopic) + 13];
  strcat(topic, settings.mqttTopic);
  strcat(topic, suffix);

Maybe this is the problem. You are concatenating a string to an uninitialized local variable. Use strncpy() for the first field and strncat() for the second:

   const byte topicSize =  sizeof settings.mqttTopic + 13;
  char topic[topicSize];
  strncpy(topic, settings.mqttTopic, topicSize);
  strncat(topic, suffix, topicSize - strlen(topic));

Unless you WANT to have your program full of buffer overflow security bugs, use strncpy() and strncat() instead of strcpy() and strcat(). Note that strncat() doesn't take the buffer size, just the space remaining in the buffer.

thank you John. That's very helpful.

a side question: would sprintf give the same result?

const byte topicSize =  sizeof settings.mqttTopic + 13;
char topic[topicSize]
sprintf(topic, "%s%s", settings.mqttTopic, "some/other/1");

jpadie:
a side question: would sprintf give the same result?

const byte topicSize =  sizeof settings.mqttTopic + 13;

char topic[topicSize];
sprintf(topic, "%s%s", settings.mqttTopic, "some/other/1");

Same BAD result as the original code or same GOOD result as the fixed code?

It would not produce the same BAD result in that you aren't treating an uninitialized local character array as a string. If "settings.mqttTopic" is a character array containing a properly terminated string then it should be safe. Even more safe would be 'snprintf()' which, again, would keep someone from overflowing your buffer:

const byte topicSize =  sizeof settings.mqttTopic + 13;
char topic[topicSize];
snprintf(topic, topicSize, "%s%s", settings.mqttTopic, "some/other/1");

NOTE: The printf() library is LARGE because it has to be able to parse any format string. Unless you already need it for more complicated formatting, or you have far more processor time and memory than you need, I would not recommend it for simple concatenation.

thank you John.