aboutgitcodelistschat:MatrixIRC
diff options
context:
space:
mode:
authorAlice Frosi <afrosi@redhat.com>2023-05-09 10:38:21 +0200
committerAlice Frosi <afrosi@redhat.com>2023-05-09 15:58:23 +0200
commit0977f0876af186975d3861c53b8431a80a27fa83 (patch)
tree9ace2c75d0389175591e8f3b9cf7e6589330514f
parent384d09cd3d2e62bae19b59b615bc57b7a23d0b0a (diff)
downloadseitan-0977f0876af186975d3861c53b8431a80a27fa83.tar
seitan-0977f0876af186975d3861c53b8431a80a27fa83.tar.gz
seitan-0977f0876af186975d3861c53b8431a80a27fa83.tar.bz2
seitan-0977f0876af186975d3861c53b8431a80a27fa83.tar.lz
seitan-0977f0876af186975d3861c53b8431a80a27fa83.tar.xz
seitan-0977f0876af186975d3861c53b8431a80a27fa83.tar.zst
seitan-0977f0876af186975d3861c53b8431a80a27fa83.zip
gluten: check limits
Add bounds checking: - if offset is larger then the maximum per offset type - if memcpy is reading/writing inside gluten
-rw-r--r--common/gluten.h48
-rw-r--r--operations.c16
-rw-r--r--tests/unit/Makefile10
-rw-r--r--tests/unit/test_errors.c118
-rw-r--r--tests/unit/testutil.h2
5 files changed, 183 insertions, 11 deletions
diff --git a/common/gluten.h b/common/gluten.h
index b1723ca..d1d3c61 100644
--- a/common/gluten.h
+++ b/common/gluten.h
@@ -26,7 +26,30 @@ extern struct seccomp_data anonymous_seccomp_data;
MAX(MAX(MAX(DATA_SIZE, RO_DATA_SIZE), INST_MAX), \
ARRAY_SIZE(anonymous_seccomp_data.args))
-#define seccomp_offset_args(x) (sizeof(uint64_t) / sizeof(uint16_t)) * (x)
+#define check_gluten_limits(g, v, size) \
+ do { \
+ struct gluten_offset off = { v.type, v.offset + (size) }; \
+ if (!is_offset_valid(off)) \
+ die(" invalid offset: %d", off.offset); \
+ } while (0)
+
+#define gluten_write(g, dst, src) \
+ do { \
+ void *p = gluten_write_ptr((g), (dst)); \
+ check_gluten_limits((g), (dst), sizeof((src))); \
+ if (p == NULL) \
+ die(" invalid type of offset"); \
+ memcpy(p, &(src), sizeof(src)); \
+ } while (0)
+
+#define gluten_read(s, g, dst, src, size) \
+ do { \
+ const void *p = gluten_ptr((s), (g), (src)); \
+ check_gluten_limits((g), (src), (size)); \
+ if (p == NULL) \
+ die(" invalid type of offset"); \
+ memcpy(&(dst), p, (size)); \
+ } while (0)
enum gluten_offset_type {
OFFSET_RO_DATA = 0,
@@ -195,6 +218,22 @@ struct gluten {
BUILD_BUG_ON(INST_SIZE < INST_MAX * sizeof(struct op))
+static inline bool is_offset_valid(const struct gluten_offset x)
+{
+ switch (x.type) {
+ case OFFSET_DATA:
+ return x.offset < DATA_SIZE;
+ case OFFSET_RO_DATA:
+ return x.offset < RO_DATA_SIZE;
+ case OFFSET_INSTRUCTION:
+ return x.offset < INST_SIZE;
+ case OFFSET_SECCOMP_DATA:
+ return x.offset < 6;
+ default:
+ return false;
+ }
+}
+
#ifdef COOKER
static inline void *gluten_ptr(struct gluten *g, const struct gluten_offset x)
#else
@@ -202,7 +241,8 @@ static inline void *gluten_write_ptr(struct gluten *g,
const struct gluten_offset x)
#endif
{
- /* TODO: Boundary checks */
+ if (!is_offset_valid(x))
+ die(" invalid offset: %d", x.offset);
switch (x.type) {
case OFFSET_DATA:
@@ -223,6 +263,9 @@ static inline const void *gluten_ptr(const struct seccomp_data *s,
struct gluten *g,
const struct gluten_offset x)
{
+ if (!is_offset_valid(x))
+ die(" invalid offset: %d", x.offset);
+
switch (x.type) {
case OFFSET_DATA:
return g->data + x.offset;
@@ -237,5 +280,4 @@ static inline const void *gluten_ptr(const struct seccomp_data *s,
}
}
#endif
-
#endif /* COMMON_GLUTEN_H */
diff --git a/operations.c b/operations.c
index 870ecf1..bf03ab8 100644
--- a/operations.c
+++ b/operations.c
@@ -177,6 +177,7 @@ int op_load(const struct seccomp_notif *req, int notifier, struct gluten *g,
ret = -1;
goto out;
}
+ check_gluten_limits(&g, load->dst, load->size);
if (pread(fd, gluten_write_ptr(g, load->dst), load->size, *src) < 0) {
perror("pread");
return -1;
@@ -232,7 +233,7 @@ int op_call(const struct seccomp_notif *req, int notifier, struct gluten *g,
* reference
*/
if (op->has_ret)
- memcpy(gluten_write_ptr(g, op->ret), &c.ret, sizeof(c.ret));
+ gluten_write(g, op->ret, c.ret);
return 0;
}
@@ -263,7 +264,7 @@ int op_return(const struct seccomp_notif *req, int notifier, struct gluten *g,
resp.flags = 0;
resp.error = 0;
- memcpy(&resp.val, gluten_ptr(NULL, g, op->val), sizeof(resp.val));
+ gluten_read(NULL, g, resp.val, op->val, sizeof(resp.val));
if (send_target(&resp, notifier) == -1)
return -1;
@@ -299,10 +300,8 @@ static int do_inject(const struct seccomp_notif *req, int notifier,
resp.newfd_flags = 0;
resp.id = req->id;
- memcpy(&resp.newfd, gluten_ptr(NULL, g, op->new_fd),
- sizeof(resp.newfd));
- memcpy(&resp.srcfd, gluten_ptr(NULL, g, op->new_fd),
- sizeof(resp.srcfd));
+ gluten_read(NULL, g, resp.newfd, op->new_fd, sizeof(resp.newfd));
+ gluten_read(NULL, g, resp.srcfd, op->old_fd, sizeof(resp.srcfd));
if (atomic)
resp.flags |= SECCOMP_ADDFD_FLAG_SEND;
@@ -351,8 +350,9 @@ int op_resolve_fd(const struct seccomp_notif *req, int notifier,
(void)notifier;
- memcpy(&path, gluten_ptr(NULL, g, op->path), op->path_size);
- memcpy(&fd, gluten_ptr(NULL, g, op->fd), sizeof(fd));
+
+ gluten_read(NULL, g, path, op->path, sizeof(op->path_size));
+ gluten_read(NULL, g, fd, op->fd, sizeof(fd));
snprintf(fdpath, PATH_MAX, "/proc/%d/fd/%d", req->pid, fd);
if ((nbytes = readlink(fdpath, buf, op->path_size)) < 0) {
diff --git a/tests/unit/Makefile b/tests/unit/Makefile
index 202e198..966ae7d 100644
--- a/tests/unit/Makefile
+++ b/tests/unit/Makefile
@@ -23,6 +23,10 @@ HEADERS_OP := $(COMMON_DIR)/gluten.h $(OP_DIR)/operations.h \
$(COMMON_DIR)/common.h testutil.h $(COMMON_DIR)/util.h
SRCS_OP := $(COMMON_DIR)/common.c $(OP_DIR)/operations.c util.c $(COMMON_DIR)/util.c
+HEADERS_ERROR := $(COMMON_DIR)/gluten.h $(OP_DIR)/operations.h \
+ $(COMMON_DIR)/common.h testutil.h $(COMMON_DIR)/util.h
+SRCS_ERROR := $(COMMON_DIR)/common.c $(OP_DIR)/operations.c util.c $(COMMON_DIR)/util.c
+
TARGET := $(shell $(CC) -dumpmachine)
TARGET_ARCH := $(shell echo $(TARGET) | cut -f1 -d- | tr [A-Z] [a-z])
TARGET_ARCH := $(shell echo $(TARGET_ARCH) | sed 's/powerpc/ppc/')
@@ -67,5 +71,11 @@ build-operations: test_operations.c $(SRCS_OP) $(HEADERS_OP)
test-operations: build-operations
./operations
+build-error-checks: test_errors.c $(SRCS_ERROR) $(HEADERS_ERROR)
+ $(CC) $(CFLAGS) -o error_checks $(SRCS_ERROR) \
+ test_errors.c
+
+test-error-checks: build-error-checks
+ ./error_checks
clean:
rm -f operations op-call filter filter-build
diff --git a/tests/unit/test_errors.c b/tests/unit/test_errors.c
new file mode 100644
index 0000000..d00d42e
--- /dev/null
+++ b/tests/unit/test_errors.c
@@ -0,0 +1,118 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+#include <sys/syscall.h>
+
+#include <check.h>
+
+#include "operations.h"
+#include "common/common.h"
+#include "common/gluten.h"
+#include "testutil.h"
+
+static void setup_error_check()
+{
+ at = mmap(NULL, sizeof(struct args_target), PROT_READ | PROT_WRITE,
+ MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+ at->check_fd = false;
+ at->nr = __NR_getpid;
+ at->install_filter = install_notification_filter;
+ setup();
+}
+
+struct gluten_offset test_max_size_data[] = {
+ { OFFSET_DATA, DATA_SIZE },
+ { OFFSET_RO_DATA, RO_DATA_SIZE },
+ { OFFSET_SECCOMP_DATA, 6 },
+ { OFFSET_INSTRUCTION, INST_SIZE },
+};
+
+START_TEST(test_bound_check)
+{
+ struct op ops[] = {
+ { OP_RETURN, { { 0 } } },
+ };
+ ops[0].op.ret.val.offset = test_max_size_data[_i].offset;
+ ops[0].op.ret.val.type = test_max_size_data[_i].type;
+
+ eval(&gluten, ops, &req, notifyfd);
+}
+
+START_TEST(test_write_op_return)
+{
+ struct op ops[] = {
+ { OP_CALL,
+ { .call = { .nr = __NR_getpid,
+ .has_ret = true,
+ .ret = { OFFSET_DATA, DATA_SIZE - 1 } } } },
+ };
+
+ eval(&gluten, ops, &req, notifyfd);
+}
+
+START_TEST(test_write_op_load)
+{
+ char a[30];
+ struct op ops[] = {
+ { OP_LOAD,
+ { .load = { { OFFSET_SECCOMP_DATA, 1 },
+ { OFFSET_DATA, DATA_SIZE - 1 },
+ sizeof(a) } } },
+ };
+
+ eval(&gluten, ops, &req, notifyfd);
+}
+
+START_TEST(test_read_op_return)
+{
+ struct op ops[] = {
+ { OP_RETURN, { { 0 } } },
+ };
+ ops[0].op.ret.val.offset = test_max_size_data[_i].offset - 1;
+ ops[0].op.ret.val.type = test_max_size_data[_i].type;
+
+ eval(&gluten, ops, &req, notifyfd);
+}
+
+Suite *error_suite(void)
+{
+ Suite *s;
+ TCase *bounds, *gwrite, *gread;
+
+ s = suite_create("Error handling");
+
+ bounds = tcase_create("bound checks");
+ tcase_add_loop_exit_test(bounds, test_bound_check, EXIT_FAILURE, 0,
+ sizeof(test_max_size_data) /
+ sizeof(test_max_size_data[0]));
+ suite_add_tcase(s, bounds);
+
+ gwrite = tcase_create("write gluten");
+ tcase_add_checked_fixture(gwrite, setup_error_check, teardown);
+ tcase_add_exit_test(gwrite, test_write_op_return, EXIT_FAILURE);
+ tcase_add_exit_test(gwrite, test_write_op_load, EXIT_FAILURE);
+ suite_add_tcase(s, gwrite);
+
+ gread = tcase_create("read gluten");
+ tcase_add_loop_exit_test(gread, test_read_op_return, EXIT_FAILURE, 0,
+ sizeof(test_max_size_data) /
+ sizeof(test_max_size_data[0]));
+ suite_add_tcase(s, gread);
+
+ return s;
+}
+
+int main(void)
+{
+ int no_failed = 0;
+ Suite *s;
+ SRunner *runner;
+
+ s = error_suite();
+ runner = srunner_create(s);
+
+ srunner_run_all(runner, CK_VERBOSE);
+ no_failed = srunner_ntests_failed(runner);
+ srunner_free(runner);
+ return (no_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
diff --git a/tests/unit/testutil.h b/tests/unit/testutil.h
index 45fe08f..ec881c7 100644
--- a/tests/unit/testutil.h
+++ b/tests/unit/testutil.h
@@ -20,6 +20,8 @@
static inline void *test_gluten_write_ptr(struct gluten *g,
const struct gluten_offset x)
{
+ ck_assert_msg(is_offset_valid(x), "offset out of bounds");
+
switch (x.type) {
case OFFSET_DATA:
return (char *)g->data + x.offset;