add more checks in lower layers
to avoid segmentation faults, out of bounds access and integer overflow make parseable for splint suppress splint warnings about datatype cc_t fix type of return value for read() add checks before accessing tty
This commit is contained in:
Stefan Wahren
parent
a640295d1b
commit
858aea33ab
@ -9,6 +9,7 @@
|
||||
//------------------------------------------------------------------------------
|
||||
|
||||
#include <unistd.h>
|
||||
#include <limits.h>
|
||||
#include <fcntl.h>
|
||||
|
||||
#include <sys/types.h>
|
||||
@ -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);
|
||||
@ -105,46 +106,49 @@ mbus_serial_set_baudrate(mbus_handle *handle, long baudrate)
|
||||
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
@ -216,6 +227,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)
|
||||
{
|
||||
fprintf(stderr, "%s: mbus_frame_pack failed\n", __PRETTY_FUNCTION__);
|
||||
@ -262,7 +279,8 @@ 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)
|
||||
{
|
||||
@ -270,6 +288,12 @@ mbus_serial_recv_frame(mbus_handle *handle, mbus_frame *frame)
|
||||
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)
|
||||
@ -303,6 +333,12 @@ mbus_serial_recv_frame(mbus_handle *handle, mbus_frame *frame)
|
||||
}
|
||||
}
|
||||
|
||||
if (len > (SSIZE_MAX-nread))
|
||||
{
|
||||
// avoid overflow
|
||||
return MBUS_RECV_RESULT_ERROR;
|
||||
}
|
||||
|
||||
len += nread;
|
||||
|
||||
} while ((remaining = mbus_parse(frame, buff, len)) > 0);
|
||||
|
||||
@ -9,6 +9,7 @@
|
||||
//------------------------------------------------------------------------------
|
||||
|
||||
#include <unistd.h>
|
||||
#include <limits.h>
|
||||
#include <fcntl.h>
|
||||
|
||||
#include <sys/socket.h>
|
||||
@ -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);
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user