Skip to content
This repository has been archived by the owner on Jun 27, 2019. It is now read-only.

Commit

Permalink
flow/file: Fix segmentation fault while sending error packet
Browse files Browse the repository at this point in the history
Functions taking a printf-style formatting string should never be
allowed to receive user-supplied strings.  In this case, a string has
been sent to `sol_flow_send_error_packet()` after being formatted by
`snprintf()` -- with `%s` and all -- causing the program to crash.

Modify the prototype for `sol_flow_send_error_packet()` so that it's
declared with `__attribute__((printf))` and fix all problematic places
in the tree with help from the compiler.

Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>
  • Loading branch information
lpereira committed Sep 30, 2015
1 parent 72ca37c commit 346bfcf
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 30 deletions.
15 changes: 14 additions & 1 deletion src/lib/flow/include/sol-flow.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include <stdint.h>
#include <time.h>

#include "sol-util.h"
#include "sol-flow-packet.h"

#ifdef __cplusplus
Expand Down Expand Up @@ -149,7 +150,19 @@ int sol_flow_send_packet(struct sol_flow_node *src, uint16_t src_port, struct so
/* Works exaclty as @sol_flow_send_packet(). This function is a helper
* to send an error packet.
*/
int sol_flow_send_error_packet(struct sol_flow_node *src, int code, const char *msg_fmt, ...);
int sol_flow_send_error_packet(struct sol_flow_node *src, int code, const char *msg_fmt, ...) SOL_ATTR_PRINTF(3, 4);

static inline int sol_flow_send_error_packet_errno(struct sol_flow_node *src, int code)
{
if (code < 0)
code = -code;
return sol_flow_send_error_packet(src, code, "%s (errno %d)", sol_util_strerrora(code), code);
}

static inline int sol_flow_send_error_packet_str(struct sol_flow_node *src, int code, const char *str)
{
return sol_flow_send_error_packet(src, code, "%s", str);
}

/* Helper functions to create and send packets of specific types. They
* work like sol_flow_send_packet(), but besides creating, sending and
Expand Down
2 changes: 1 addition & 1 deletion src/modules/flow-metatype/js/js.c
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ send_error_packet(duk_context *ctx)
return 0;
}

r = sol_flow_send_error_packet(node, value_code, value_msg);
r = sol_flow_send_error_packet_str(node, value_code, value_msg);
if (r < 0)
duk_error(ctx, DUK_ERR_ERROR, "Couldn't send error packet.");

Expand Down
4 changes: 2 additions & 2 deletions src/modules/flow/converter/string-format.c
Original file line number Diff line number Diff line change
Expand Up @@ -2097,7 +2097,7 @@ get_integer_field(struct string_converter *mdata,
break;
default:
sol_flow_send_error_packet(mdata->node, EINVAL,
"Field index %d does not exist for integer type", index);
"Field index %zd does not exist for integer type", index);
}
}

Expand Down Expand Up @@ -2152,7 +2152,7 @@ get_float_field(struct string_converter *mdata,
break;
default:
sol_flow_send_error_packet(mdata->node, EINVAL,
"Field index %d does not exist for float type", index);
"Field index %zd does not exist for float type", index);
}
}

Expand Down
10 changes: 4 additions & 6 deletions src/modules/flow/file/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,8 @@ file_reader_load(struct file_reader_data *mdata)

reader = sol_file_reader_open(mdata->path);
if (!reader) {
char errmsg[1024];
snprintf(errmsg, sizeof(errmsg), "Could not load \"%s\": %s",
mdata->path, sol_util_strerrora(errno));
sol_flow_send_error_packet(mdata->node, errno, errmsg);
sol_flow_send_error_packet(mdata->node, errno,
"Could not load \"%s\": %s", mdata->path, sol_util_strerrora(errno));
return -errno;
}
slice = sol_file_reader_get_all(reader);
Expand Down Expand Up @@ -280,7 +278,7 @@ file_writer_worker_thread_setup(void *data)
char *msg;
mdata->error = errno;
msg = sol_util_strerrora(errno);
sol_flow_send_error_packet(mdata->node, mdata->error, msg);
sol_flow_send_error_packet_str(mdata->node, mdata->error, msg);
SOL_WRN("could not open '%s': %s", mdata->path, msg);
return false;
}
Expand Down Expand Up @@ -336,7 +334,7 @@ file_writer_worker_thread_iterate(void *data)
msg = sol_util_strerrora(errno);
SOL_WRN("could not write %zd bytes to fd=%d (%s): %s",
todo, mdata->fd, mdata->path, msg);
sol_flow_send_error_packet(mdata->node, mdata->error, msg);
sol_flow_send_error_packet_str(mdata->node, mdata->error, msg);
return false;
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/modules/flow/filter-repeated/filter-repeated.c
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,12 @@ error_filter(struct sol_flow_node *node, void *data, uint16_t port, uint16_t con
free(mdata->msg);
mdata->msg = strdup(in_value);
if (!mdata->msg) {
sol_flow_send_error_packet(node, errno, sol_util_strerrora(errno));
sol_flow_send_error_packet_errno(node, errno);
return -errno;
}
mdata->code = code_value;

return sol_flow_send_error_packet(node, code_value, in_value);
return sol_flow_send_error_packet_str(node, code_value, in_value);
}

static int
Expand Down Expand Up @@ -258,7 +258,7 @@ string_filter(struct sol_flow_node *node, void *data, uint16_t port, uint16_t co
free(mdata->value);
mdata->value = strdup(in_value);
if (!mdata->value) {
sol_flow_send_error_packet(node, errno, sol_util_strerrora(errno));
sol_flow_send_error_packet_errno(node, errno);
return -errno;
}

Expand Down
6 changes: 3 additions & 3 deletions src/modules/flow/float/float.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ operator_process(struct sol_flow_node *node, void *data, uint16_t port, uint16_t
r = sol_flow_packet_get_drange(packet, &value);

if (r < 0) {
sol_flow_send_error_packet(node, -r, sol_util_strerrora(-r));
sol_flow_send_error_packet_errno(node, -r);
return r;
}

Expand Down Expand Up @@ -174,15 +174,15 @@ multiple_operator_process(struct sol_flow_node *node, void *data, uint16_t port,
result = alloca(sizeof(*result));
if (!result) {
r = ENOMEM;
sol_flow_send_error_packet(node, r, sol_util_strerrora(r));
sol_flow_send_error_packet_errno(node, r);
return -r;
}
*result = mdata->var[i];
continue;
} else {
r = type->func(result, &mdata->var[i], result);
if (r < 0) {
sol_flow_send_error_packet(node, -r, sol_util_strerrora(-r));
sol_flow_send_error_packet_errno(node, -r);
return r;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/modules/flow/iio/nodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ reader_cb(void *data, struct sol_iio_device *device)
return;

error:
sol_flow_send_error_packet(mdata->node, EIO, errmsg);
sol_flow_send_error_packet_str(mdata->node, EIO, errmsg);
SOL_WRN("%s", errmsg);
}

Expand Down Expand Up @@ -178,7 +178,7 @@ gyroscope_tick(struct sol_flow_node *node, void *data, uint16_t port, uint16_t c
return 0;

error:
sol_flow_send_error_packet(node, EIO, errmsg);
sol_flow_send_error_packet(node, EIO, "%s", errmsg);
SOL_WRN("%s", errmsg);

return -EIO;
Expand Down
6 changes: 3 additions & 3 deletions src/modules/flow/int/int.c
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ operator_process(struct sol_flow_node *node, void *data, uint16_t port, uint16_t
sol_flow_node_get_type(node);
r = type->func(&mdata->var0, &mdata->var1, &value);
if (r < 0) {
sol_flow_send_error_packet(node, -r, sol_util_strerrora(-r));
sol_flow_send_error_packet_errno(node, -r);
return r;
}

Expand Down Expand Up @@ -459,15 +459,15 @@ multiple_operator_process(struct sol_flow_node *node, void *data, uint16_t port,
result = alloca(sizeof(*result));
if (!result) {
r = ENOMEM;
sol_flow_send_error_packet(node, r, sol_util_strerrora(r));
sol_flow_send_error_packet_errno(node, r);
return -r;
}
*result = mdata->var[i];
continue;
} else {
r = type->func(result, &mdata->var[i], result);
if (r < 0) {
sol_flow_send_error_packet(node, -r, sol_util_strerrora(-r));
sol_flow_send_error_packet_errno(node, -r);
return r;
}
}
Expand Down
14 changes: 7 additions & 7 deletions src/modules/flow/string/string-icu.c
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ string_concat(struct sol_flow_node *node,
r = utf8_from_icu_str_slice(dest, len, &final, &err);
if (r < 0) {
free(dest);
sol_flow_send_error_packet(node, -r, u_errorName(err));
sol_flow_send_error_packet_str(node, -r, u_errorName(err));
return r;
}

Expand Down Expand Up @@ -379,7 +379,7 @@ slice_do(struct string_slice_data *mdata)
r = utf8_from_icu_str_slice((const UChar *)slice.data, slice.len,
&outstr, &err);
if (r < 0) {
sol_flow_send_error_packet(mdata->node, -r, u_errorName(err));
sol_flow_send_error_packet_str(mdata->node, -r, u_errorName(err));
return r;
}

Expand All @@ -406,7 +406,7 @@ string_slice_input(struct sol_flow_node *node,
mdata->str = NULL;
r = icu_str_from_utf8(in_value, &mdata->str, &err);
if (r < 0) {
sol_flow_send_error_packet(mdata->node, -r, u_errorName(err));
sol_flow_send_error_packet_str(mdata->node, -r, u_errorName(err));
return r;
}

Expand Down Expand Up @@ -817,7 +817,7 @@ string_change_case(struct sol_flow_node *node,

r = icu_str_from_utf8(value, &u_orig, &err);
if (r < 0) {
sol_flow_send_error_packet(node, -r, u_errorName(err));
sol_flow_send_error_packet_str(node, -r, u_errorName(err));
return -errno;
}

Expand All @@ -835,7 +835,7 @@ string_change_case(struct sol_flow_node *node,
if (U_FAILURE(err) && err != U_BUFFER_OVERFLOW_ERROR) {
errno = EINVAL;
free(u_orig);
sol_flow_send_error_packet(node, errno, u_errorName(err));
sol_flow_send_error_packet_str(node, errno, u_errorName(err));
return -errno;
}
u_lower = calloc(u_changed_sz + 1, sizeof(*u_lower));
Expand All @@ -852,15 +852,15 @@ string_change_case(struct sol_flow_node *node,
errno = EINVAL;
free(u_orig);
free(u_lower);
sol_flow_send_error_packet(node, errno, u_errorName(err));
sol_flow_send_error_packet_str(node, errno, u_errorName(err));
return -errno;
}

r = utf8_from_icu_str_slice(u_lower, u_changed_sz, &final, &err);
if (r < 0) {
free(u_orig);
free(u_lower);
sol_flow_send_error_packet(node, -r, u_errorName(err));
sol_flow_send_error_packet(node, -r, "%s", u_errorName(err));
return r;
}

Expand Down
2 changes: 1 addition & 1 deletion src/modules/flow/switcher/switcher.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ error_forward(struct sol_flow_node *node, void *data, uint16_t port, uint16_t co
r = sol_flow_packet_get_error(packet, &code_value, &msg);
SOL_INT_CHECK(r, < 0, r);

return sol_flow_send_error_packet(node, code_value, msg);
return sol_flow_send_error_packet(node, code_value, "%s", msg);
}

static int
Expand Down
3 changes: 2 additions & 1 deletion src/modules/flow/test/boolean-generator.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ timer_tick(void *data)
out_packet = false;
} else {
sol_flow_send_error_packet(node, ECANCELED,
"Unknown sample: %c. Option 'sequence' must be composed by 'T' and/or 'F' chars.");
"Unknown sample: %c. Option 'sequence' must be composed by 'T' and/or 'F' chars.",
*mdata->it);
return false;
}

Expand Down

0 comments on commit 346bfcf

Please sign in to comment.