code.ossystems Code Review - openembedded-core.git/blob

   1 From 7ea36eeece56b59f98e469934e4c20b4da043346 Mon Sep 17 00:00:00 2001
   2 From: Doran Moppert <dmoppert@redhat.com>
   3 Date: Thu, 11 May 2017 11:42:54 -0400
   4 Subject: [PATCH] rpcbind: pair all svc_getargs() calls with svc_freeargs() to
   5  avoid memory leak
   6
   7 This patch is to address CVE-2017-8779 "rpcbomb" in rpcbind, discussed
   8 at [1], [2], [3].  The last link suggests this issue is actually a bug
   9 in rpcbind, which led me here.
  10
  11 The leak caused by the reproducer at [4] appears to come from
  12 rpcb_service_4(), in the case where svc_getargs() returns false and the
  13 function had an early return, rather than passing through the cleanup
  14 path at done:, as would otherwise occur.
  15
  16 It also addresses a couple of other locations where the same fault seems
  17 to exist, though I haven't been able to exercise those.  I hope someone
  18 more intimate with rpc(3) can confirm my understanding is correct, and
  19 that I haven't introduced any new bugs.
  20
  21 Without this patch, using the reproducer (and variants) repeatedly
  22 against rpcbind with a numBytes argument of 1_000_000_000, /proc/$(pidof
  23 rpcbind)/status reports VmSize increase of 976564 kB each call, and
  24 VmRSS increase of around 260 kB every 33 calls - the specific numbers
  25 are probably an artifact of my rhel/glibc version.  With the patch,
  26 there is a small (~50 kB) VmSize increase with the first message, but
  27 thereafter both VmSize and VmRSS remain steady.
  28
  29 [1]: http://seclists.org/oss-sec/2017/q2/209
  30 [2]: https://bugzilla.redhat.com/show_bug.cgi?id=1448124
  31 [3]: https://sourceware.org/ml/libc-alpha/2017-05/msg00129.html
  32 [4]: https://github.com/guidovranken/rpcbomb/
  33
  34
  35 CVE: CVE-2017-8779
  36 Upstream-Status: Backport
  37
  38 Signed-off-by: Fan Xin <fan.xin@jp.fujitsu.com>
  39 ---
  40  src/pmap_svc.c     | 56 +++++++++++++++++++++++++++++++++++++++++++++---------
  41  src/rpcb_svc.c     |  2 +-
  42  src/rpcb_svc_4.c   |  2 +-
  43  src/rpcb_svc_com.c |  8 ++++++++
  44  4 files changed, 57 insertions(+), 11 deletions(-)
  45
  46 diff --git a/src/pmap_svc.c b/src/pmap_svc.c
  47 index 4c744fe..e926cdc 100644
  48 --- a/src/pmap_svc.c
  49 +++ b/src/pmap_svc.c
  50 @@ -175,6 +175,7 @@ pmapproc_change(struct svc_req *rqstp /*__unused*/, SVCXPRT *xprt, unsigned long
  51         long ans;
  52         uid_t uid;
  53         char uidbuf[32];
  54 +       int rc = TRUE;
  55
  56         /*
  57          * Can't use getpwnam here. We might end up calling ourselves
  58 @@ -194,7 +195,8 @@ pmapproc_change(struct svc_req *rqstp /*__unused*/, SVCXPRT *xprt, unsigned long
  59
  60         if (!svc_getargs(xprt, (xdrproc_t) xdr_pmap, (char *)&reg)) {
  61                 svcerr_decode(xprt);
  62 -               return (FALSE);
  63 +               rc = FALSE;
  64 +               goto done;
  65         }
  66  #ifdef RPCBIND_DEBUG
  67         if (debugging)
  68 @@ -205,7 +207,8 @@ pmapproc_change(struct svc_req *rqstp /*__unused*/, SVCXPRT *xprt, unsigned long
  69
  70         if (!check_access(xprt, op, reg.pm_prog, PMAPVERS)) {
  71                 svcerr_weakauth(xprt);
  72 -               return (FALSE);
  73 +               rc = (FALSE);
  74 +               goto done;
  75         }
  76
  77         rpcbreg.r_prog = reg.pm_prog;
  78 @@ -258,7 +261,16 @@ done_change:
  79                 rpcbs_set(RPCBVERS_2_STAT, ans);
  80         else
  81                 rpcbs_unset(RPCBVERS_2_STAT, ans);
  82 -       return (TRUE);
  83 +done:
  84 +       if (!svc_freeargs(xprt, (xdrproc_t) xdr_pmap, (char *)&reg)) {
  85 +               if (debugging) {
  86 +                       /*(void) xlog(LOG_DEBUG, "unable to free arguments\n");*/
  87 +                       if (doabort) {
  88 +                               rpcbind_abort();
  89 +                       }
  90 +               }
  91 +       }
  92 +       return (rc);
  93  }
  94
  95  /* ARGSUSED */
  96 @@ -272,15 +284,18 @@ pmapproc_getport(struct svc_req *rqstp /*__unused*/, SVCXPRT *xprt)
  97  #ifdef RPCBIND_DEBUG
  98         char *uaddr;
  99  #endif
 100 +       int rc = TRUE;
 101
 102         if (!svc_getargs(xprt, (xdrproc_t) xdr_pmap, (char *)&reg)) {
 103                 svcerr_decode(xprt);
 104 -               return (FALSE);
 105 +               rc = FALSE;
 106 +               goto done;
 107         }
 108
 109         if (!check_access(xprt, PMAPPROC_GETPORT, reg.pm_prog, PMAPVERS)) {
 110                 svcerr_weakauth(xprt);
 111 -               return FALSE;
 112 +               rc = FALSE;
 113 +               goto done;
 114         }
 115
 116  #ifdef RPCBIND_DEBUG
 117 @@ -330,21 +345,34 @@ pmapproc_getport(struct svc_req *rqstp /*__unused*/, SVCXPRT *xprt)
 118                 pmap_ipprot2netid(reg.pm_prot) ?: "<unknown>",
 119                 port ? udptrans : "");
 120
 121 -       return (TRUE);
 122 +done:
 123 +       if (!svc_freeargs(xprt, (xdrproc_t) xdr_pmap, (char *)&reg)) {
 124 +               if (debugging) {
 125 +                       /* (void) xlog(LOG_DEBUG, "unable to free arguments\n");*/
 126 +                       if (doabort) {
 127 +                               rpcbind_abort();
 128 +                       }
 129 +               }
 130 +       }
 131 +       return (rc);
 132  }
 133
 134  /* ARGSUSED */
 135  static bool_t
 136  pmapproc_dump(struct svc_req *rqstp /*__unused*/, SVCXPRT *xprt)
 137  {
 138 +       int rc = TRUE;
 139 +
 140         if (!svc_getargs(xprt, (xdrproc_t)xdr_void, NULL)) {
 141                 svcerr_decode(xprt);
 142 -               return (FALSE);
 143 +               rc = FALSE;
 144 +               goto done;
 145         }
 146
 147         if (!check_access(xprt, PMAPPROC_DUMP, 0, PMAPVERS)) {
 148                 svcerr_weakauth(xprt);
 149 -               return FALSE;
 150 +               rc = FALSE;
 151 +               goto done;
 152         }
 153
 154         if ((!svc_sendreply(xprt, (xdrproc_t) xdr_pmaplist_ptr,
 155 @@ -354,7 +382,17 @@ pmapproc_dump(struct svc_req *rqstp /*__unused*/, SVCXPRT *xprt)
 156                         rpcbind_abort();
 157                 }
 158         }
 159 -       return (TRUE);
 160 +
 161 +done:
 162 +       if (!svc_freeargs(xprt, (xdrproc_t) xdr_pmap, (char *)NULL)) {
 163 +               if (debugging) {
 164 +                       /*(void) xlog(LOG_DEBUG, "unable to free arguments\n");*/
 165 +                       if (doabort) {
 166 +                               rpcbind_abort();
 167 +                       }
 168 +               }
 169 +       }
 170 +       return (rc);
 171  }
 172
 173  int pmap_netid2ipprot(const char *netid)
 174 diff --git a/src/rpcb_svc.c b/src/rpcb_svc.c
 175 index 709e3fb..091f530 100644
 176 --- a/src/rpcb_svc.c
 177 +++ b/src/rpcb_svc.c
 178 @@ -166,7 +166,7 @@ rpcb_service_3(struct svc_req *rqstp, SVCXPRT *transp)
 179                 svcerr_decode(transp);
 180                 if (debugging)
 181                         (void) xlog(LOG_DEBUG, "rpcbind: could not decode");
 182 -               return;
 183 +               goto done;
 184         }
 185
 186         if (rqstp->rq_proc == RPCBPROC_SET
 187 diff --git a/src/rpcb_svc_4.c b/src/rpcb_svc_4.c
 188 index 5094879..eebbbbe 100644
 189 --- a/src/rpcb_svc_4.c
 190 +++ b/src/rpcb_svc_4.c
 191 @@ -218,7 +218,7 @@ rpcb_service_4(struct svc_req *rqstp, SVCXPRT *transp)
 192                 svcerr_decode(transp);
 193                 if (debugging)
 194                         (void) xlog(LOG_DEBUG, "rpcbind: could not decode\n");
 195 -               return;
 196 +               goto done;
 197         }
 198
 199         if (rqstp->rq_proc == RPCBPROC_SET
 200 diff --git a/src/rpcb_svc_com.c b/src/rpcb_svc_com.c
 201 index 5862c26..cb63afd 100644
 202 --- a/src/rpcb_svc_com.c
 203 +++ b/src/rpcb_svc_com.c
 204 @@ -927,6 +927,14 @@ error:
 205         if (call_msg.rm_xid != 0)
 206                 (void) free_slot_by_xid(call_msg.rm_xid);
 207  out:
 208 +       if (!svc_freeargs(transp, (xdrproc_t) xdr_rmtcall_args, (char *) &a)) {
 209 +               if (debugging) {
 210 +                       (void) xlog(LOG_DEBUG, "unable to free arguments\n");
 211 +                       if (doabort) {
 212 +                               rpcbind_abort();
 213 +                       }
 214 +               }
 215 +       }
 216         if (local_uaddr)
 217                 free(local_uaddr);
 218         if (buf_alloc)
 219 --
 220 1.9.1
 221