releases/2.6.18.5/netfilter-missed-and-reordered-checks-in-arp-ip-ip6-_tables.patch

   1 From stable-bounces@linux.kernel.org  Tue Nov 21 02:39:12 2006
   2 Message-ID: <4562D63F.5060207@trash.net>
   3 Date: Tue, 21 Nov 2006 11:34:39 +0100
   4 From: Patrick McHardy <kaber@trash.net>
   5 To: Chris Wright <chrisw@sous-sol.org>
   6 Cc: dim@openvz.org, dev@openvz.org, davem@davemloft.net, stable@kernel.org
   7 Subject: NETFILTER: Missed and reordered checks in {arp,ip,ip6}_tables
   8
   9 Backport fix for missing ruleset validation in {arp,ip,ip6}_tables
  10 and a fix on top which fixes a regression in the first patch.
  11
  12 There is a number of issues in parsing user-provided table in
  13 translate_table(). Malicious user with CAP_NET_ADMIN may crash system by
  14 passing special-crafted table to the *_tables.
  15
  16 The first issue is that mark_source_chains() function is called before entry
  17 content checks. In case of standard target, mark_source_chains() function
  18 uses t->verdict field in order to determine new position. But the check, that
  19 this field leads no further, than the table end, is in check_entry(), which
  20 is called later, than mark_source_chains().
  21
  22 The second issue, that there is no check that target_offset points inside
  23 entry. If so, *_ITERATE_MATCH macro will follow further, than the entry
  24 ends. As a result, we'll have oops or memory disclosure.
  25
  26 And the third issue, that there is no check that the target is completely
  27 inside entry. Results are the same, as in previous issue.
  28
  29 Upstream commit 590bdf7fd2292b47c428111cb1360e312eff207e introduced a
  30 regression in match/target hook validation. mark_source_chains builds
  31 a bitmask for each rule representing the hooks it can be reached from,
  32 which is then used by the matches and targets to make sure they are
  33 only called from valid hooks. The patch moved the match/target specific
  34 validation before the mark_source_chains call, at which point the mask
  35 is always zero.
  36
  37 This patch returns back to the old order and moves the standard checks
  38 to mark_source_chains. This allows to get rid of a special case for
  39 standard targets as a nice side-effect.
  40
  41 Signed-off-by: Patrick McHardy <kaber@trash.net>
  42 Signed-off-by: Chris Wright <chrisw@sous-sol.org>
  43
  44 ---
  45 commit 1cfcb663c5a6d8b4b1172ff481af1b597bc8b54e
  46 tree 61c5b135ee292681f38945a3549cb9005aec1d7c
  47 parent b2ab160e1a3a1eb3fcc80132d8d7db5687a7a113
  48 author Patrick McHardy <kaber@trash.net> Tue, 21 Nov 2006 11:17:03 +0100
  49 committer Patrick McHardy <kaber@trash.net> Tue, 21 Nov 2006 11:24:51 +0100
  50
  51  net/ipv4/netfilter/arp_tables.c |   37 +++++++++++++---------
  52  net/ipv4/netfilter/ip_tables.c  |   65 +++++++++++++++++++---------------------
  53  net/ipv6/netfilter/ip6_tables.c |   53 ++++++++++++++------------------
  54  3 files changed, 77 insertions(+), 78 deletions(-)
  55
  56 --- linux-2.6.18.4.orig/net/ipv4/netfilter/arp_tables.c
  57 +++ linux-2.6.18.4/net/ipv4/netfilter/arp_tables.c
  58 @@ -380,6 +380,13 @@ static int mark_source_chains(struct xt_
  59                             && unconditional(&e->arp)) {
  60                                 unsigned int oldpos, size;
  61
  62 +                               if (t->verdict < -NF_MAX_VERDICT - 1) {
  63 +                                       duprintf("mark_source_chains: bad "
  64 +                                               "negative verdict (%i)\n",
  65 +                                                               t->verdict);
  66 +                                       return 0;
  67 +                               }
  68 +
  69                                 /* Return: backtrack through the last
  70                                  * big jump.
  71                                  */
  72 @@ -409,6 +416,14 @@ static int mark_source_chains(struct xt_
  73                                 if (strcmp(t->target.u.user.name,
  74                                            ARPT_STANDARD_TARGET) == 0
  75                                     && newpos >= 0) {
  76 +                                       if (newpos > newinfo->size -
  77 +                                               sizeof(struct arpt_entry)) {
  78 +                                               duprintf("mark_source_chains: "
  79 +                                                       "bad verdict (%i)\n",
  80 +                                                               newpos);
  81 +                                               return 0;
  82 +                                       }
  83 +
  84                                         /* This a jump; chase it. */
  85                                         duprintf("Jump rule %u -> %u\n",
  86                                                  pos, newpos);
  87 @@ -431,8 +446,6 @@ static int mark_source_chains(struct xt_
  88  static inline int standard_check(const struct arpt_entry_target *t,
  89                                  unsigned int max_offset)
  90  {
  91 -       struct arpt_standard_target *targ = (void *)t;
  92 -
  93         /* Check standard info. */
  94         if (t->u.target_size
  95             != ARPT_ALIGN(sizeof(struct arpt_standard_target))) {
  96 @@ -442,18 +455,6 @@ static inline int standard_check(const s
  97                 return 0;
  98         }
  99
 100 -       if (targ->verdict >= 0
 101 -           && targ->verdict > max_offset - sizeof(struct arpt_entry)) {
 102 -               duprintf("arpt_standard_check: bad verdict (%i)\n",
 103 -                        targ->verdict);
 104 -               return 0;
 105 -       }
 106 -
 107 -       if (targ->verdict < -NF_MAX_VERDICT - 1) {
 108 -               duprintf("arpt_standard_check: bad negative verdict (%i)\n",
 109 -                        targ->verdict);
 110 -               return 0;
 111 -       }
 112         return 1;
 113  }
 114
 115 @@ -471,7 +472,13 @@ static inline int check_entry(struct arp
 116                 return -EINVAL;
 117         }
 118
 119 +       if (e->target_offset + sizeof(struct arpt_entry_target) > e->next_offset)
 120 +               return -EINVAL;
 121 +
 122         t = arpt_get_target(e);
 123 +       if (e->target_offset + t->u.target_size > e->next_offset)
 124 +               return -EINVAL;
 125 +
 126         target = try_then_request_module(xt_find_target(NF_ARP, t->u.user.name,
 127                                                         t->u.user.revision),
 128                                          "arpt_%s", t->u.user.name);
 129 @@ -641,7 +648,7 @@ static int translate_table(const char *n
 130
 131         if (ret != 0) {
 132                 ARPT_ENTRY_ITERATE(entry0, newinfo->size,
 133 -                                  cleanup_entry, &i);
 134 +                               cleanup_entry, &i);
 135                 return ret;
 136         }
 137
 138 --- linux-2.6.18.4.orig/net/ipv4/netfilter/ip_tables.c
 139 +++ linux-2.6.18.4/net/ipv4/netfilter/ip_tables.c
 140 @@ -404,6 +404,13 @@ mark_source_chains(struct xt_table_info
 141                             && unconditional(&e->ip)) {
 142                                 unsigned int oldpos, size;
 143
 144 +                               if (t->verdict < -NF_MAX_VERDICT - 1) {
 145 +                                       duprintf("mark_source_chains: bad "
 146 +                                               "negative verdict (%i)\n",
 147 +                                                               t->verdict);
 148 +                                       return 0;
 149 +                               }
 150 +
 151                                 /* Return: backtrack through the last
 152                                    big jump. */
 153                                 do {
 154 @@ -441,6 +448,13 @@ mark_source_chains(struct xt_table_info
 155                                 if (strcmp(t->target.u.user.name,
 156                                            IPT_STANDARD_TARGET) == 0
 157                                     && newpos >= 0) {
 158 +                                       if (newpos > newinfo->size -
 159 +                                               sizeof(struct ipt_entry)) {
 160 +                                               duprintf("mark_source_chains: "
 161 +                                                       "bad verdict (%i)\n",
 162 +                                                               newpos);
 163 +                                               return 0;
 164 +                                       }
 165                                         /* This a jump; chase it. */
 166                                         duprintf("Jump rule %u -> %u\n",
 167                                                  pos, newpos);
 168 @@ -474,27 +488,6 @@ cleanup_match(struct ipt_entry_match *m,
 169  }
 170
 171  static inline int
 172 -standard_check(const struct ipt_entry_target *t,
 173 -              unsigned int max_offset)
 174 -{
 175 -       struct ipt_standard_target *targ = (void *)t;
 176 -
 177 -       /* Check standard info. */
 178 -       if (targ->verdict >= 0
 179 -           && targ->verdict > max_offset - sizeof(struct ipt_entry)) {
 180 -               duprintf("ipt_standard_check: bad verdict (%i)\n",
 181 -                        targ->verdict);
 182 -               return 0;
 183 -       }
 184 -       if (targ->verdict < -NF_MAX_VERDICT - 1) {
 185 -               duprintf("ipt_standard_check: bad negative verdict (%i)\n",
 186 -                        targ->verdict);
 187 -               return 0;
 188 -       }
 189 -       return 1;
 190 -}
 191 -
 192 -static inline int
 193  check_match(struct ipt_entry_match *m,
 194             const char *name,
 195             const struct ipt_ip *ip,
 196 @@ -552,12 +545,18 @@ check_entry(struct ipt_entry *e, const c
 197                 return -EINVAL;
 198         }
 199
 200 +       if (e->target_offset + sizeof(struct ipt_entry_target) > e->next_offset)
 201 +               return -EINVAL;
 202 +
 203         j = 0;
 204         ret = IPT_MATCH_ITERATE(e, check_match, name, &e->ip, e->comefrom, &j);
 205         if (ret != 0)
 206                 goto cleanup_matches;
 207
 208         t = ipt_get_target(e);
 209 +       ret = -EINVAL;
 210 +       if (e->target_offset + t->u.target_size > e->next_offset)
 211 +                       goto cleanup_matches;
 212         target = try_then_request_module(xt_find_target(AF_INET,
 213                                                      t->u.user.name,
 214                                                      t->u.user.revision),
 215 @@ -575,12 +574,7 @@ check_entry(struct ipt_entry *e, const c
 216         if (ret)
 217                 goto err;
 218
 219 -       if (t->u.kernel.target == &ipt_standard_target) {
 220 -               if (!standard_check(t, size)) {
 221 -                       ret = -EINVAL;
 222 -                       goto cleanup_matches;
 223 -               }
 224 -       } else if (t->u.kernel.target->checkentry
 225 +       if (t->u.kernel.target->checkentry
 226                    && !t->u.kernel.target->checkentry(name, e, target, t->data,
 227                                                       t->u.target_size
 228                                                       - sizeof(*t),
 229 @@ -730,7 +724,7 @@ translate_table(const char *name,
 230
 231         if (ret != 0) {
 232                 IPT_ENTRY_ITERATE(entry0, newinfo->size,
 233 -                                 cleanup_entry, &i);
 234 +                               cleanup_entry, &i);
 235                 return ret;
 236         }
 237
 238 @@ -1531,6 +1525,10 @@ check_compat_entry_size_and_hooks(struct
 239                 return -EINVAL;
 240         }
 241
 242 +       if (e->target_offset + sizeof(struct compat_xt_entry_target) >
 243 +                                                               e->next_offset)
 244 +               return -EINVAL;
 245 +
 246         off = 0;
 247         entry_offset = (void *)e - (void *)base;
 248         j = 0;
 249 @@ -1540,6 +1538,9 @@ check_compat_entry_size_and_hooks(struct
 250                 goto cleanup_matches;
 251
 252         t = ipt_get_target(e);
 253 +       ret = -EINVAL;
 254 +       if (e->target_offset + t->u.target_size > e->next_offset)
 255 +                       goto cleanup_matches;
 256         target = try_then_request_module(xt_find_target(AF_INET,
 257                                                      t->u.user.name,
 258                                                      t->u.user.revision),
 259 @@ -1656,19 +1657,15 @@ static int compat_copy_entry_from_user(s
 260         if (ret)
 261                 goto err;
 262
 263 -       ret = -EINVAL;
 264 -       if (t->u.kernel.target == &ipt_standard_target) {
 265 -               if (!standard_check(t, *size))
 266 -                       goto err;
 267 -       } else if (t->u.kernel.target->checkentry
 268 +       if (t->u.kernel.target->checkentry
 269                    && !t->u.kernel.target->checkentry(name, de, target,
 270                                 t->data, t->u.target_size - sizeof(*t),
 271                                 de->comefrom)) {
 272                 duprintf("ip_tables: compat: check failed for `%s'.\n",
 273                          t->u.kernel.target->name);
 274 +               ret = -EINVAL;
 275                 goto err;
 276         }
 277 -       ret = 0;
 278   err:
 279         return ret;
 280  }
 281 --- linux-2.6.18.4.orig/net/ipv6/netfilter/ip6_tables.c
 282 +++ linux-2.6.18.4/net/ipv6/netfilter/ip6_tables.c
 283 @@ -444,6 +444,13 @@ mark_source_chains(struct xt_table_info
 284                             && unconditional(&e->ipv6)) {
 285                                 unsigned int oldpos, size;
 286
 287 +                               if (t->verdict < -NF_MAX_VERDICT - 1) {
 288 +                                       duprintf("mark_source_chains: bad "
 289 +                                               "negative verdict (%i)\n",
 290 +                                                               t->verdict);
 291 +                                       return 0;
 292 +                               }
 293 +
 294                                 /* Return: backtrack through the last
 295                                    big jump. */
 296                                 do {
 297 @@ -481,6 +488,13 @@ mark_source_chains(struct xt_table_info
 298                                 if (strcmp(t->target.u.user.name,
 299                                            IP6T_STANDARD_TARGET) == 0
 300                                     && newpos >= 0) {
 301 +                                       if (newpos > newinfo->size -
 302 +                                               sizeof(struct ip6t_entry)) {
 303 +                                               duprintf("mark_source_chains: "
 304 +                                                       "bad verdict (%i)\n",
 305 +                                                               newpos);
 306 +                                               return 0;
 307 +                                       }
 308                                         /* This a jump; chase it. */
 309                                         duprintf("Jump rule %u -> %u\n",
 310                                                  pos, newpos);
 311 @@ -514,27 +528,6 @@ cleanup_match(struct ip6t_entry_match *m
 312  }
 313
 314  static inline int
 315 -standard_check(const struct ip6t_entry_target *t,
 316 -              unsigned int max_offset)
 317 -{
 318 -       struct ip6t_standard_target *targ = (void *)t;
 319 -
 320 -       /* Check standard info. */
 321 -       if (targ->verdict >= 0
 322 -           && targ->verdict > max_offset - sizeof(struct ip6t_entry)) {
 323 -               duprintf("ip6t_standard_check: bad verdict (%i)\n",
 324 -                        targ->verdict);
 325 -               return 0;
 326 -       }
 327 -       if (targ->verdict < -NF_MAX_VERDICT - 1) {
 328 -               duprintf("ip6t_standard_check: bad negative verdict (%i)\n",
 329 -                        targ->verdict);
 330 -               return 0;
 331 -       }
 332 -       return 1;
 333 -}
 334 -
 335 -static inline int
 336  check_match(struct ip6t_entry_match *m,
 337             const char *name,
 338             const struct ip6t_ip6 *ipv6,
 339 @@ -592,12 +585,19 @@ check_entry(struct ip6t_entry *e, const
 340                 return -EINVAL;
 341         }
 342
 343 +       if (e->target_offset + sizeof(struct ip6t_entry_target) >
 344 +                                                               e->next_offset)
 345 +               return -EINVAL;
 346 +
 347         j = 0;
 348         ret = IP6T_MATCH_ITERATE(e, check_match, name, &e->ipv6, e->comefrom, &j);
 349         if (ret != 0)
 350                 goto cleanup_matches;
 351
 352         t = ip6t_get_target(e);
 353 +       ret = -EINVAL;
 354 +       if (e->target_offset + t->u.target_size > e->next_offset)
 355 +                       goto cleanup_matches;
 356         target = try_then_request_module(xt_find_target(AF_INET6,
 357                                                         t->u.user.name,
 358                                                         t->u.user.revision),
 359 @@ -615,12 +615,7 @@ check_entry(struct ip6t_entry *e, const
 360         if (ret)
 361                 goto err;
 362
 363 -       if (t->u.kernel.target == &ip6t_standard_target) {
 364 -               if (!standard_check(t, size)) {
 365 -                       ret = -EINVAL;
 366 -                       goto cleanup_matches;
 367 -               }
 368 -       } else if (t->u.kernel.target->checkentry
 369 +       if (t->u.kernel.target->checkentry
 370                    && !t->u.kernel.target->checkentry(name, e, target, t->data,
 371                                                       t->u.target_size
 372                                                       - sizeof(*t),
 373 @@ -770,7 +765,7 @@ translate_table(const char *name,
 374
 375         if (ret != 0) {
 376                 IP6T_ENTRY_ITERATE(entry0, newinfo->size,
 377 -                                 cleanup_entry, &i);
 378 +                                  cleanup_entry, &i);
 379                 return ret;
 380         }
 381
 382 @@ -780,7 +775,7 @@ translate_table(const char *name,
 383                         memcpy(newinfo->entries[i], entry0, newinfo->size);
 384         }
 385
 386 -       return ret;
 387 +       return 0;
 388  }
 389
 390  /* Gets counters. */