diff --git a/mbus/mbus-serial.c b/mbus/mbus-serial.c index 45f3983..16fe6a4 100755 --- a/mbus/mbus-serial.c +++ b/mbus/mbus-serial.c @@ -9,6 +9,7 @@ //------------------------------------------------------------------------------ #include +#include #include #include @@ -61,7 +62,7 @@ mbus_serial_connect(mbus_handle *handle) term->c_cflag |= PARENB; // No received data still OK - term->c_cc[VMIN] = 0; + term->c_cc[VMIN] = (cc_t) 0; // Wait at most 0.2 sec.Note that it starts after first received byte!! // I.e. if CMIN>0 and there are no data we would still wait forever... @@ -74,7 +75,7 @@ mbus_serial_connect(mbus_handle *handle) // For 2400Bd this means (330 + 11) / 2400 + 0.05 = 188.75 ms (added 11 bit periods to receive first byte). // I.e. timeout of 0.2s seems appropriate for 2400Bd. - term->c_cc[VTIME] = 2; // Timeout in 1/10 sec + term->c_cc[VTIME] = (cc_t) 2; // Timeout in 1/10 sec cfsetispeed(term, B2400); cfsetospeed(term, B2400); @@ -104,47 +105,50 @@ mbus_serial_set_baudrate(mbus_handle *handle, long baudrate) return -1; serial_data = (mbus_serial_data *) handle->auxdata; + + if (serial_data == NULL) + return -1; switch (baudrate) { case 300: speed = B300; - serial_data->t.c_cc[VTIME] = 12; // Timeout in 1/10 sec + serial_data->t.c_cc[VTIME] = (cc_t) 12; // Timeout in 1/10 sec break; case 600: speed = B600; - serial_data->t.c_cc[VTIME] = 6; // Timeout in 1/10 sec + serial_data->t.c_cc[VTIME] = (cc_t) 6; // Timeout in 1/10 sec break; case 1200: speed = B1200; - serial_data->t.c_cc[VTIME] = 4; // Timeout in 1/10 sec + serial_data->t.c_cc[VTIME] = (cc_t) 4; // Timeout in 1/10 sec break; case 2400: speed = B2400; - serial_data->t.c_cc[VTIME] = 2; // Timeout in 1/10 sec + serial_data->t.c_cc[VTIME] = (cc_t) 2; // Timeout in 1/10 sec break; case 4800: speed = B4800; - serial_data->t.c_cc[VTIME] = 2; // Timeout in 1/10 sec + serial_data->t.c_cc[VTIME] = (cc_t) 2; // Timeout in 1/10 sec break; case 9600: speed = B9600; - serial_data->t.c_cc[VTIME] = 1; // Timeout in 1/10 sec + serial_data->t.c_cc[VTIME] = (cc_t) 1; // Timeout in 1/10 sec break; case 19200: speed = B19200; - serial_data->t.c_cc[VTIME] = 1; // Timeout in 1/10 sec + serial_data->t.c_cc[VTIME] = (cc_t) 1; // Timeout in 1/10 sec break; case 38400: speed = B38400; - serial_data->t.c_cc[VTIME] = 1; // Timeout in 1/10 sec + serial_data->t.c_cc[VTIME] = (cc_t) 1; // Timeout in 1/10 sec break; default: @@ -197,8 +201,15 @@ mbus_serial_data_free(mbus_handle *handle) if (handle) { serial_data = (mbus_serial_data *) handle->auxdata; + + if (serial_data == NULL) + { + return; + } + free(serial_data->device); free(serial_data); + handle->auxdata = NULL; } } @@ -215,6 +226,12 @@ mbus_serial_send_frame(mbus_handle *handle, mbus_frame *frame) { return -1; } + + // Make sure serial connection is open + if (isatty(handle->fd) == 0) + { + return -1; + } if ((len = mbus_frame_pack(frame, buff, sizeof(buff))) == -1) { @@ -262,13 +279,20 @@ int mbus_serial_recv_frame(mbus_handle *handle, mbus_frame *frame) { char buff[PACKET_BUFF_SIZE]; - int len, remaining, nread, timeouts; + int remaining, timeouts; + ssize_t len, nread; if (handle == NULL || frame == NULL) { fprintf(stderr, "%s: Invalid parameter.\n", __PRETTY_FUNCTION__); return MBUS_RECV_RESULT_ERROR; } + + // Make sure serial connection is open + if (isatty(handle->fd) == 0) + { + return -1; + } memset((void *)buff, 0, sizeof(buff)); @@ -280,6 +304,12 @@ mbus_serial_recv_frame(mbus_handle *handle, mbus_frame *frame) timeouts = 0; do { + if (len + remaining > PACKET_BUFF_SIZE) + { + // avoid out of bounds access + return MBUS_RECV_RESULT_ERROR; + } + //printf("%s: Attempt to read %d bytes [len = %d]\n", __PRETTY_FUNCTION__, remaining, len); if ((nread = read(handle->fd, &buff[len], remaining)) == -1) @@ -302,6 +332,12 @@ mbus_serial_recv_frame(mbus_handle *handle, mbus_frame *frame) break; } } + + if (len > (SSIZE_MAX-nread)) + { + // avoid overflow + return MBUS_RECV_RESULT_ERROR; + } len += nread; diff --git a/mbus/mbus-tcp.c b/mbus/mbus-tcp.c index 671bebf..3490034 100755 --- a/mbus/mbus-tcp.c +++ b/mbus/mbus-tcp.c @@ -9,6 +9,7 @@ //------------------------------------------------------------------------------ #include +#include #include #include @@ -97,8 +98,15 @@ mbus_tcp_data_free(mbus_handle *handle) if (handle) { tcp_data = (mbus_tcp_data *) handle->auxdata; + + if (tcp_data == NULL) + { + return; + } + free(tcp_data->host); free(tcp_data); + handle->auxdata = NULL; } } @@ -126,6 +134,7 @@ mbus_tcp_send_frame(mbus_handle *handle, mbus_frame *frame) { unsigned char buff[PACKET_BUFF_SIZE]; int len, ret; + char error_str[128]; if (handle == NULL || frame == NULL) { @@ -134,7 +143,6 @@ mbus_tcp_send_frame(mbus_handle *handle, mbus_frame *frame) if ((len = mbus_frame_pack(frame, buff, sizeof(buff))) == -1) { - char error_str[128]; snprintf(error_str, sizeof(error_str), "%s: mbus_frame_pack failed\n", __PRETTY_FUNCTION__); mbus_error_str_set(error_str); return -1; @@ -150,7 +158,6 @@ mbus_tcp_send_frame(mbus_handle *handle, mbus_frame *frame) } else { - char error_str[128]; snprintf(error_str, sizeof(error_str), "%s: Failed to write frame to socket (ret = %d)\n", __PRETTY_FUNCTION__, ret); mbus_error_str_set(error_str); return -1; @@ -165,7 +172,8 @@ mbus_tcp_send_frame(mbus_handle *handle, mbus_frame *frame) int mbus_tcp_recv_frame(mbus_handle *handle, mbus_frame *frame) { char buff[PACKET_BUFF_SIZE]; - int len, remaining, nread; + int remaining; + ssize_t len, nread; if (handle == NULL || frame == NULL) { fprintf(stderr, "%s: Invalid parameter.\n", __PRETTY_FUNCTION__); @@ -182,6 +190,12 @@ int mbus_tcp_recv_frame(mbus_handle *handle, mbus_frame *frame) do { retry: + if (len + remaining > PACKET_BUFF_SIZE) + { + // avoid out of bounds access + return MBUS_RECV_RESULT_ERROR; + } + nread = read(handle->fd, &buff[len], remaining); switch (nread) { case -1: @@ -199,6 +213,12 @@ retry: mbus_error_str_set("M-Bus tcp transport layer connection closed by remote host."); return MBUS_RECV_RESULT_RESET; default: + if (len > (SSIZE_MAX-nread)) + { + // avoid overflow + return MBUS_RECV_RESULT_ERROR; + } + len += nread; } } while ((remaining = mbus_parse(frame, buff, len)) > 0);