expr: fix buffer overflows in data value setters

The data value setters memcpy() to a fixed-size buffer, but its very easy
to make nft pass too-larger values.  Example:
  @th,160,1272 gt 0

ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60b000[..]

Truncate the copy instead of corrupting the heap.
This needs additional fixes on nft side to reject such statements with a
proper error message.

Signed-off-by: Florian Westphal <fw@strlen.de>

diff --git a/include/data_reg.h b/include/data_reg.h
index 6d2dc66858bf8198392261086757028159d2f129..5ee7080daef06d666391ec86cc1ec74671408e16 100644 (file)
--- a/include/data_reg.h
+++ b/include/data_reg.h
@@ -37,4 +37,6 @@ struct nlattr;
 int nftnl_parse_data(union nftnl_data_reg *data, struct nlattr *attr, int *type);
 void nftnl_free_verdict(const union nftnl_data_reg *data);
 
+int nftnl_data_cpy(union nftnl_data_reg *dreg, const void *src, uint32_t len);
+
 #endif

diff --git a/src/expr/bitwise.c b/src/expr/bitwise.c
index 2d272335e377234ee098ac1991c079f6c9b64ba3..e5dba827f3d932eb47b11f0d98ea8acaa78ea2d0 100644 (file)
--- a/src/expr/bitwise.c
+++ b/src/expr/bitwise.c
@@ -51,17 +51,11 @@ nftnl_expr_bitwise_set(struct nftnl_expr *e, uint16_t type,
                memcpy(&bitwise->len, data, sizeof(bitwise->len));
                break;
        case NFTNL_EXPR_BITWISE_MASK:
-               memcpy(&bitwise->mask.val, data, data_len);
-               bitwise->mask.len = data_len;
-               break;
+               return nftnl_data_cpy(&bitwise->mask, data, data_len);
        case NFTNL_EXPR_BITWISE_XOR:
-               memcpy(&bitwise->xor.val, data, data_len);
-               bitwise->xor.len = data_len;
-               break;
+               return nftnl_data_cpy(&bitwise->xor, data, data_len);
        case NFTNL_EXPR_BITWISE_DATA:
-               memcpy(&bitwise->data.val, data, data_len);
-               bitwise->data.len = data_len;
-               break;
+               return nftnl_data_cpy(&bitwise->data, data, data_len);
        default:
                return -1;
        }

diff --git a/src/expr/cmp.c b/src/expr/cmp.c
index f9d15bba3b0fa7e280d5af974d45dbbc34ca0876..1d396e83411ab7819281672e8205f1deb87d8930 100644 (file)
--- a/src/expr/cmp.c
+++ b/src/expr/cmp.c
@@ -42,9 +42,7 @@ nftnl_expr_cmp_set(struct nftnl_expr *e, uint16_t type,
                memcpy(&cmp->op, data, sizeof(cmp->op));
                break;
        case NFTNL_EXPR_CMP_DATA:
-               memcpy(&cmp->data.val, data, data_len);
-               cmp->data.len = data_len;
-               break;
+               return nftnl_data_cpy(&cmp->data, data, data_len);
        default:
                return -1;
        }

diff --git a/src/expr/data_reg.c b/src/expr/data_reg.c
index 2633a775c90cc634f4637e552b80a8c7d1a8e53d..690b23dbad6c86d7098dba3583d558a7885b6753 100644 (file)
--- a/src/expr/data_reg.c
+++ b/src/expr/data_reg.c
@@ -217,3 +217,17 @@ void nftnl_free_verdict(const union nftnl_data_reg *data)
                break;
        }
 }
+
+int nftnl_data_cpy(union nftnl_data_reg *dreg, const void *src, uint32_t len)
+{
+       int ret = 0;
+
+       if (len > sizeof(dreg->val)) {
+               len = sizeof(dreg->val);
+               ret = -1;
+       }
+
+       memcpy(dreg->val, src, len);
+       dreg->len = len;
+       return ret;
+}

diff --git a/src/expr/immediate.c b/src/expr/immediate.c
index 5d477a8b421763b33d1a7f81d45cfd2af1377228..f56aa8fd69997898fc4a5a12967e9a56cda1b3a0 100644 (file)
--- a/src/expr/immediate.c
+++ b/src/expr/immediate.c
@@ -36,9 +36,7 @@ nftnl_expr_immediate_set(struct nftnl_expr *e, uint16_t type,
                memcpy(&imm->dreg, data, sizeof(imm->dreg));
                break;
        case NFTNL_EXPR_IMM_DATA:
-               memcpy(&imm->data.val, data, data_len);
-               imm->data.len = data_len;
-               break;
+               return nftnl_data_cpy(&imm->data, data, data_len);
        case NFTNL_EXPR_IMM_VERDICT:
                memcpy(&imm->data.verdict, data, sizeof(imm->data.verdict));
                break;

diff --git a/src/expr/range.c b/src/expr/range.c
index 473add86e4b45b2478b4c9cfa1b841c0de632b9b..5a30e48fde928cd54d393c179f3c24a710f1230b 100644 (file)
--- a/src/expr/range.c
+++ b/src/expr/range.c
@@ -40,13 +40,9 @@ static int nftnl_expr_range_set(struct nftnl_expr *e, uint16_t type,
                memcpy(&range->op, data, sizeof(range->op));
                break;
        case NFTNL_EXPR_RANGE_FROM_DATA:
-               memcpy(&range->data_from.val, data, data_len);
-               range->data_from.len = data_len;
-               break;
+               return nftnl_data_cpy(&range->data_from, data, data_len);
        case NFTNL_EXPR_RANGE_TO_DATA:
-               memcpy(&range->data_to.val, data, data_len);
-               range->data_to.len = data_len;
-               break;
+               return nftnl_data_cpy(&range->data_to, data, data_len);
        default:
                return -1;
        }