Brad Bishop | 6e60e8b | 2018-02-01 10:27:11 -0500 | [diff] [blame] | 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 *)®)) { |
| 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 *)®)) { |
| 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 *)®)) { |
| 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 *)®)) { |
| 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 | |