On 2020-01-06, at 10:28:42 +0100, Pablo Neira Ayuso wrote:
> On Tue, Dec 24, 2019 at 11:14:28PM +0000, Jeremy Sowden wrote:
> > expr_evaluate_binop calls expr_set_context for shift expressions to
> > set the context data-type to `integer`. This clobbers the
> > byte-order of the context, resulting in unexpected conversions to
> > NBO. For example:
> >
> > $ sudo nft flush ruleset
> > $ sudo nft add table t
> > $ sudo nft add chain t c '{ type filter hook output priority mangle; }'
> > $ sudo nft add rule t c oif lo tcp dport ssh ct mark set '0x10 | 0xe'
> > $ sudo nft add rule t c oif lo tcp dport ssh ct mark set '0xf << 1'
> > $ sudo nft list table t
> > table ip t {
> > chain c {
> > type filter hook output priority mangle; policy accept;
> > oif "lo" tcp dport 22 ct mark set 0x0000001e
> > oif "lo" tcp dport 22 ct mark set 0x1e000000
> > }
> > }
> >
> > Replace it with a call to __expr_set_context in order to preserve host
> > endianness.
> >
> > Signed-off-by: Jeremy Sowden <jeremy@xxxxxxxxxx>
> > ---
> > src/evaluate.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Change-log:
> >
> > In v1, I just dropped the expr_set_context call; however, it is needed
> > to ensure that the right operand has integer type. Instead, I now
> > change it to __expr_set_context in order to ensure that the byte-order
> > is correct.
> >
> > diff --git a/src/evaluate.c b/src/evaluate.c
> > index a865902c0fc7..43637e1cf6c8 100644
> > --- a/src/evaluate.c
> > +++ b/src/evaluate.c
> > @@ -1145,7 +1145,8 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
> > left = op->left;
> >
> > if (op->op == OP_LSHIFT || op->op == OP_RSHIFT)
> > - expr_set_context(&ctx->ectx, &integer_type, ctx->ectx.len);
> > + __expr_set_context(&ctx->ectx, &integer_type,
> > + BYTEORDER_HOST_ENDIAN, ctx->ectx.len, 0);
>
> tests/py spews a few warnings after this patch.
I'll take a look.
Cheers.
J.
Attachment:
signature.asc
Description: PGP signature
