stephane / libmodbus

A Modbus library for Linux, Mac OS, FreeBSD and Windows
http://libmodbus.org
GNU Lesser General Public License v2.1
3.46k stars 1.75k forks source link

modbus_new_tcp() doesn't appear to handle NULL for the _ip address argument #307

Closed jfieldhouse closed 8 years ago

jfieldhouse commented 8 years ago

The documentation for the function modbus_new_tcp() says: ... The _modbus_newtcp() function shall allocate and initialize a modbus_t structure to communicate with a Modbus TCP IPv4 server.

The ip argument specifies the IP address of the server to which the client wants etablish a connection. A NULL value can be used to listen any addresses in server mode. ...

When I use NULL though, I get a sigsegv fault and when I look at the source code in modbus-tcp.c and modbus.c, it doesn't look like there is any check for passing NULL in as the ip argument. The ip argument is passed to another function called strlcpy, which fails when it tries to operate on memory location zero.

stephane commented 8 years ago

IP address is checked in master: https://github.com/stephane/libmodbus/blob/master/src/modbus-tcp.c#L811

Which version did you use?

jfieldhouse commented 8 years ago

I'm implementing a Modbus slave

jfieldhouse commented 8 years ago

I believe i downloaded v3.0.6 and the source code for modbus-tcp is different than what you are showing:

modbus_t* modbus_new_tcp(const char ip, int port) { modbus_t ctx; modbus_tcp_t *ctx_tcp; size_t dest_size; size_t ret_size;

if defined(OS_BSD)

/\* MSG_NOSIGNAL is unsupported on *BSD so we install an ignore
   handler for SIGPIPE. */
struct sigaction sa;
sa.sa_handler = SIG_IGN;
if (sigaction(SIGPIPE, &sa, NULL) < 0) {
    /* The debug flag can't be set here... */
    fprintf(stderr, "Coud not install SIGPIPE handler.\n");
    return NULL;
}

endif

ctx = (modbus_t *) malloc(sizeof(modbus_t));
_modbus_init_common(ctx);

/* Could be changed after to reach a remote serial Modbus device */
ctx->slave = MODBUS_TCP_SLAVE;

ctx->backend = &(_modbus_tcp_backend);

ctx->backend_data = (modbus_tcp_t *) malloc(sizeof(modbus_tcp_t));
ctx_tcp = (modbus_tcp_t *)ctx->backend_data;

dest_size = sizeof(char) * 16;
ret_size = strlcpy(ctx_tcp->ip, ip, dest_size);
if (ret_size == 0) {
    fprintf(stderr, "The IP string is empty\n");
    modbus_free(ctx);
    errno = EINVAL;
    return NULL;
}

if (ret_size >= dest_size) {
    fprintf(stderr, "The IP string has been truncated\n");
    modbus_free(ctx);
    errno = EINVAL;
    return NULL;
}

ctx_tcp->port = port;

return ctx;

}

jfieldhouse commented 8 years ago

It looks like you've only implemented that ability in the latest version of the library. Is that correct?

jfieldhouse commented 8 years ago

I see now that I was referring to the documentation for the latest code branch, but using v3.0.6 source code. Hence the disconnect I was seeing. I'll update to 3.1.2. Thanks.

stephane commented 8 years ago

Don't forget to close the issue, please.