Skip to content

Buffer Overflow during Timesync sprintf() #92

Description

@evandowning

Describe the bug
Neuralert sends JSON data via MQTT, but does not perform bounds-checking on buffers. The timesync buffer size is equal to the buffer it gets stored in. However, sprintf() adds more content to the buffer, which isn’t bounds-checked. If the timesync string is sufficiently large, it will overrun the buffer.

To Reproduce
Identified statically:

  • // timesync is structured as: "2025.02.12 17:30:05 (GMT +0:00) 000029405"
    #define MAX_TIMESYNC_LENGTH 80
  • unsigned char str[80], str2[40]; // temp working strings for assembling
    int16_t Xvalue;
    int16_t Yvalue;
    int16_t Zvalue;
    char device_id[10];
    char timesync[MAX_TIMESYNC_LENGTH];
    int tx_thres;
    #ifdef __TIME64__
    __time64_t now;
    #else
    time_t now;
    #endif /* __TIME64__ */
    struct tm;
    char nowStr[20];
    // get data from user memory
    if (take_semaphore(&User_semaphore)) {
    PRINTF("\n Neuralert: [%s] error taking user semaphore",__func__);
    PRINTF("\n Neuralert: [%s] transmit %d:%d unsuccessful", __func__, transmission, sequence);
    return pdTRUE;
    } else {
    strcpy(device_id, pUserData->Device_ID);
    strcpy(timesync, pUserData->MQTT_timesync_current_time_str);
    tx_thres = pUserData->ACCEL_transmit_threshold;
    xSemaphoreGive(User_semaphore);
    }
    /*
    * JSON preamble
    */
    strcpy(mqttMessage,"{\r\n\t\"state\":\r\n\t{\r\n\t\t\"reported\":\r\n\t\t{\r\n");
    /*
    * MAC address of device - stored in retention memory
    * during the bootup event
    */
    sprintf(str,"\t\t\t\"id\": \"%s\",\r\n", device_id);
    strcat(mqttMessage,str);
    /* New timesync field added 1/26/23 per ECO approved by Neuralert
    * Format:
    *
    * The current first JSON packet has this format:
    * { "state": { "reported": { "id": "EB345A", "meta": {},
    * "bat": 140,
    * "accX": [6 6 6 6 6 6 6 6 6 6 6 6 5 6 6 6 6 6 6 6 5 6 6 6 6 6 6 6 6 6 6 6 6 6 6 6 5 6 6 6 6 6 6 6 6 6 6 6 6 6 6 6 6
    *
    * "timesync": "2023.01.17 12:53:55 (GMT 00:00) 0656741",
    *
    * where the data consists of the current date and time in “local” time, as configured when WIFI is set up.
    * The last field (0656741) is an internal timestamp in milliseconds since power-on that corresponds to the
    * current local time. This will make it possible to align timestamps from different devices.
    *
    * Note that the “current” local date and time is that returned from an SNTP server on the Internet
    * and is subject to internet lag and internal processing times. None of which matter practically speaking.
    *
    */
    sprintf(str,"\t\t\t\"timesync\": \"%s\",\r\n", timesync);

Expected behavior
Check the length of each string before concatenating it onto the buffer.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions