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