lkml.org 
[lkml]   [2020]   [Jan]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.4 134/191] netfilter: ebtables: compat: reject all padding in matches/watchers
    Date
    From: Florian Westphal <fw@strlen.de>

    commit e608f631f0ba5f1fc5ee2e260a3a35d13107cbfe upstream.

    syzbot reported following splat:

    BUG: KASAN: vmalloc-out-of-bounds in size_entry_mwt net/bridge/netfilter/ebtables.c:2063 [inline]
    BUG: KASAN: vmalloc-out-of-bounds in compat_copy_entries+0x128b/0x1380 net/bridge/netfilter/ebtables.c:2155
    Read of size 4 at addr ffffc900004461f4 by task syz-executor267/7937

    CPU: 1 PID: 7937 Comm: syz-executor267 Not tainted 5.5.0-rc1-syzkaller #0
    size_entry_mwt net/bridge/netfilter/ebtables.c:2063 [inline]
    compat_copy_entries+0x128b/0x1380 net/bridge/netfilter/ebtables.c:2155
    compat_do_replace+0x344/0x720 net/bridge/netfilter/ebtables.c:2249
    compat_do_ebt_set_ctl+0x22f/0x27e net/bridge/netfilter/ebtables.c:2333
    [..]

    Because padding isn't considered during computation of ->buf_user_offset,
    "total" is decremented by fewer bytes than it should.

    Therefore, the first part of

    if (*total < sizeof(*entry) || entry->next_offset < sizeof(*entry))

    will pass, -- it should not have. This causes oob access:
    entry->next_offset is past the vmalloced size.

    Reject padding and check that computed user offset (sum of ebt_entry
    structure plus all individual matches/watchers/targets) is same
    value that userspace gave us as the offset of the next entry.

    Reported-by: syzbot+f68108fed972453a0ad4@syzkaller.appspotmail.com
    Fixes: 81e675c227ec ("netfilter: ebtables: add CONFIG_COMPAT support")
    Signed-off-by: Florian Westphal <fw@strlen.de>
    Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    net/bridge/netfilter/ebtables.c | 33 ++++++++++++++++-----------------
    1 file changed, 16 insertions(+), 17 deletions(-)

    --- a/net/bridge/netfilter/ebtables.c
    +++ b/net/bridge/netfilter/ebtables.c
    @@ -1867,7 +1867,7 @@ static int ebt_buf_count(struct ebt_entr
    }

    static int ebt_buf_add(struct ebt_entries_buf_state *state,
    - void *data, unsigned int sz)
    + const void *data, unsigned int sz)
    {
    if (state->buf_kern_start == NULL)
    goto count_only;
    @@ -1901,7 +1901,7 @@ enum compat_mwt {
    EBT_COMPAT_TARGET,
    };

    -static int compat_mtw_from_user(struct compat_ebt_entry_mwt *mwt,
    +static int compat_mtw_from_user(const struct compat_ebt_entry_mwt *mwt,
    enum compat_mwt compat_mwt,
    struct ebt_entries_buf_state *state,
    const unsigned char *base)
    @@ -1979,22 +1979,23 @@ static int compat_mtw_from_user(struct c
    /* return size of all matches, watchers or target, including necessary
    * alignment and padding.
    */
    -static int ebt_size_mwt(struct compat_ebt_entry_mwt *match32,
    +static int ebt_size_mwt(const struct compat_ebt_entry_mwt *match32,
    unsigned int size_left, enum compat_mwt type,
    struct ebt_entries_buf_state *state, const void *base)
    {
    + const char *buf = (const char *)match32;
    int growth = 0;
    - char *buf;

    if (size_left == 0)
    return 0;

    - buf = (char *) match32;
    -
    - while (size_left >= sizeof(*match32)) {
    + do {
    struct ebt_entry_match *match_kern;
    int ret;

    + if (size_left < sizeof(*match32))
    + return -EINVAL;
    +
    match_kern = (struct ebt_entry_match *) state->buf_kern_start;
    if (match_kern) {
    char *tmp;
    @@ -2031,22 +2032,18 @@ static int ebt_size_mwt(struct compat_eb
    if (match_kern)
    match_kern->match_size = ret;

    - /* rule should have no remaining data after target */
    - if (type == EBT_COMPAT_TARGET && size_left)
    - return -EINVAL;
    -
    match32 = (struct compat_ebt_entry_mwt *) buf;
    - }
    + } while (size_left);

    return growth;
    }

    /* called for all ebt_entry structures. */
    -static int size_entry_mwt(struct ebt_entry *entry, const unsigned char *base,
    +static int size_entry_mwt(const struct ebt_entry *entry, const unsigned char *base,
    unsigned int *total,
    struct ebt_entries_buf_state *state)
    {
    - unsigned int i, j, startoff, new_offset = 0;
    + unsigned int i, j, startoff, next_expected_off, new_offset = 0;
    /* stores match/watchers/targets & offset of next struct ebt_entry: */
    unsigned int offsets[4];
    unsigned int *offsets_update = NULL;
    @@ -2132,11 +2129,13 @@ static int size_entry_mwt(struct ebt_ent
    return ret;
    }

    - startoff = state->buf_user_offset - startoff;
    + next_expected_off = state->buf_user_offset - startoff;
    + if (next_expected_off != entry->next_offset)
    + return -EINVAL;

    - if (WARN_ON(*total < startoff))
    + if (*total < entry->next_offset)
    return -EINVAL;
    - *total -= startoff;
    + *total -= entry->next_offset;
    return 0;
    }


    \
     
     \ /
      Last update: 2020-01-02 23:58    [W:2.617 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site