[PATCH 1/1] fsidd: require root credentials on abstract socket

[PATCH 1/1] fsidd: require root credentials on abstract socket

[Olga Kornievskaia]

fsidd listens on an abstract UNIX domain socket which has no
filesystem-level access control.  Any local unprivileged user can
connect and issue mutating commands (get_or_create_fsidnum) that
grow the reexport database without limit, degrading mountd
performance.

Add a SO_PEERCRED check to reject connections from non-root
clients.  The only legitimate client is mountd, which runs as root.

Fixes: 6fd2732d ("export: Add fsidd")
Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
 support/reexport/fsidd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/support/reexport/fsidd.c b/support/reexport/fsidd.c
index 9b80b451..c714386e 100644
--- a/support/reexport/fsidd.c
+++ b/support/reexport/fsidd.c
@@ -7,6 +7,7 @@
 #include <dlfcn.h>
 #endif
 #include <event2/event.h>
+#include <sys/socket.h>
 #include <sys/un.h>
 #include <unistd.h>
 
@@ -147,6 +148,8 @@ static void srv_cb(evutil_socket_t fd, short ev, void *d)
 {
 	int cl = accept4(fd, NULL, NULL, SOCK_NONBLOCK);
 	struct event *client_ev;
+	struct ucred cred;
+	socklen_t clen = sizeof(cred);
 
 	if (cl == -1) {
 		if (errno == EMFILE || errno == ENFILE || errno == ENOMEM ||
@@ -162,6 +165,12 @@ static void srv_cb(evutil_socket_t fd, short ev, void *d)
 	(void)ev;
 	(void)d;
 
+	if (getsockopt(cl, SOL_SOCKET, SO_PEERCRED, &cred, &clen) < 0 ||
+	    cred.uid != 0) {
+		close(cl);
+		return;
+	}
+
 	client_ev = event_new(evbase, cl, EV_READ | EV_PERSIST | EV_CLOSED, client_cb, event_self_cbarg());
 	if (!client_ev || event_add(client_ev, NULL) == -1) {
 		if (client_ev)
-- 
2.52.0

[PATCH 1/3] libtirpc: Fix use-after-free in xdr_pmaplist() XDR_FREE path

[Olga Kornievskaia]

The XDR_FREE loop saved a pointer to the pml_next field inside the
struct being freed (next = &((*rp)->pml_next)), then dereferenced it
on the next iteration. This reads freed memory.

Adopt the same pattern xdr_rpcblist_ptr() already uses: save the
value of the next pointer before freeing, then point rp at a stack
copy.

Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
 src/pmap_prot2.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/pmap_prot2.c b/src/pmap_prot2.c
index bbac74f..7611bd5 100644
--- a/src/pmap_prot2.c
+++ b/src/pmap_prot2.c
@@ -88,7 +88,8 @@ xdr_pmaplist(XDR *xdrs, struct pmaplist **rp)
 	 */
 	bool_t more_elements;
 	int freeing;
-	struct pmaplist **next	= NULL; /* pacify gcc */
+	struct pmaplist *next = NULL;
+	struct pmaplist *next_copy;
 
 	assert(xdrs != NULL);
 	assert(rp != NULL);
@@ -107,11 +108,16 @@ xdr_pmaplist(XDR *xdrs, struct pmaplist **rp)
 		 * before we free the current object ...
 		 */
 		if (freeing)
-			next = &((*rp)->pml_next); 
+			next = (*rp)->pml_next;
 		if (! xdr_reference(xdrs, (caddr_t *)rp,
 		    (u_int)sizeof(struct pmaplist), (xdrproc_t)xdr_pmap))
 			return (FALSE);
-		rp = (freeing) ? next : &((*rp)->pml_next);
+		if (freeing) {
+			next_copy = next;
+			rp = &next_copy;
+		} else {
+			rp = &((*rp)->pml_next);
+		}
 	}
 }
 
-- 
2.52.0

[PATCH 2/3] libtirpc: limit XDR decode node count

[Olga Kornievskaia]

xdr_pmaplist(), xdr_rpcblist_ptr() and xdr_rpcb_entry_list_ptr()
decode linked lists with no upper bound on element count. A malicious
server can stream arbitrarily many list nodes, forcing unbounded heap
allocation in any client that decodes the response (rpcinfo,
rpcb_getmaps() callers).

Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
 src/pmap_prot2.c |  9 ++++++++-
 src/rpcb_prot.c  | 16 ++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/pmap_prot2.c b/src/pmap_prot2.c
index 7611bd5..d043f34 100644
--- a/src/pmap_prot2.c
+++ b/src/pmap_prot2.c
@@ -39,6 +39,8 @@
 #include <rpc/xdr.h>
 #include <rpc/pmap_prot.h>
 
+/* Cap decoded list length to prevent memory exhaustion from malicious data */
+#define PMAPLIST_DECODE_MAX_NODES 1024
 
 /*
  * What is going on with linked lists? (!)
@@ -87,7 +89,7 @@ xdr_pmaplist(XDR *xdrs, struct pmaplist **rp)
 	 * xdr_bool when the direction is XDR_DECODE.
 	 */
 	bool_t more_elements;
-	int freeing;
+	int freeing, node_count = 0;
 	struct pmaplist *next = NULL;
 	struct pmaplist *next_copy;
 
@@ -102,6 +104,11 @@ xdr_pmaplist(XDR *xdrs, struct pmaplist **rp)
 			return (FALSE);
 		if (! more_elements)
 			return (TRUE);  /* we are done */
+		if (xdrs->x_op == XDR_DECODE &&
+			++node_count > PMAPLIST_DECODE_MAX_NODES) {
+			return (FALSE);
+		}
+
 		/*
 		 * the unfortunate side effect of non-recursion is that in
 		 * the case of freeing we must remember the next object
diff --git a/src/rpcb_prot.c b/src/rpcb_prot.c
index 809feb0..0b0574f 100644
--- a/src/rpcb_prot.c
+++ b/src/rpcb_prot.c
@@ -89,6 +89,9 @@ xdr_rpcb(
  * serialize the rpcb elements.
  */
 
+/* Cap decoded list length to prevent memory exhaustion from malicious data */
+#define RPCBLIST_DECODE_MAX_NODES 1024
+
 bool_t
 xdr_rpcblist_ptr(
 	XDR *xdrs,
@@ -101,6 +104,7 @@ xdr_rpcblist_ptr(
 	 */
 	bool_t more_elements;
 	int freeing = (xdrs->x_op == XDR_FREE);
+	int node_count = 0;
 	rpcblist_ptr next;
 	rpcblist_ptr next_copy;
 
@@ -113,6 +117,10 @@ xdr_rpcblist_ptr(
 		if (! more_elements) {
 			return (TRUE);  /* we are done */
 		}
+		if (xdrs->x_op == XDR_DECODE &&
+		    ++node_count > RPCBLIST_DECODE_MAX_NODES) {
+			return (FALSE);
+		}
 		/*
 		 * the unfortunate side effect of non-recursion is that in
 		 * the case of freeing we must remember the next object
@@ -178,6 +186,9 @@ xdr_rpcb_entry(
 	return (TRUE);
 }
 
+/* Cap decoded list length to prevent memory exhaustion from malicious data */
+#define RPCB_ENTRY_LIST_DECODE_MAX_NODES 1024
+
 bool_t
 xdr_rpcb_entry_list_ptr(
 	XDR *xdrs,
@@ -190,6 +201,7 @@ xdr_rpcb_entry_list_ptr(
 	 */
 	bool_t more_elements;
 	int freeing = (xdrs->x_op == XDR_FREE);
+	int node_count = 0;
 	rpcb_entry_list_ptr next;
 	rpcb_entry_list_ptr next_copy;
 
@@ -202,6 +214,10 @@ xdr_rpcb_entry_list_ptr(
 		if (! more_elements) {
 			return (TRUE);  /* we are done */
 		}
+		if (xdrs->x_op == XDR_DECODE &&
+		    ++node_count > RPCB_ENTRY_LIST_DECODE_MAX_NODES) {
+			return (FALSE);
+		}
 		/*
 		 * the unfortunate side effect of non-recursion is that in
 		 * the case of freeing we must remember the next object
-- 
2.52.0

[PATCH 3/3] libtirpc: Bound maxsize in xdr_rpc_gss_unwrap_data() decode calls

[Olga Kornievskaia]

xdr_rpc_gss_unwrap_data() passes (u_int)-1 as maxsize to
xdr_rpc_gss_buf() for all three decode calls (databody_integ,
checksum, databody_priv). This disables the length check in
xdr_bytes(), allowing a fabricated length field in a small RPC
message to trigger a multi-GB allocation before GSS verification.

Replace (u_int)-1 with RPCSEC_GSS_MAX_UNWRAP (16 MB). The encode
path already uses bounded values (wrapbuf.length + RPC_SLACK_SPACE).

Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
 src/authgss_prot.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/authgss_prot.c b/src/authgss_prot.c
index c2b25db..c7de1fd 100644
--- a/src/authgss_prot.c
+++ b/src/authgss_prot.c
@@ -51,6 +51,9 @@
 /* additional space needed for encoding */
 #define RPC_SLACK_SPACE 1024
 
+/* upper bound on decoded RPCSEC_GSS wrapped payload (16 MB) */
+#define RPCSEC_GSS_MAX_UNWRAP (16U * 1024U * 1024U)
+
 bool_t
 xdr_rpc_gss_buf(XDR *xdrs, gss_buffer_t buf, u_int maxsize)
 {
@@ -227,12 +230,12 @@ xdr_rpc_gss_unwrap_data(XDR *xdrs, xdrproc_t xdr_func, caddr_t xdr_ptr,
 
 	if (svc == RPCSEC_GSS_SVC_INTEGRITY) {
 		/* Decode databody_integ. */
-		if (!xdr_rpc_gss_buf(xdrs, &databuf, (u_int)-1)) {
+		if (!xdr_rpc_gss_buf(xdrs, &databuf, RPCSEC_GSS_MAX_UNWRAP)) {
 			LIBTIRPC_DEBUG(1, ("xdr_rpc_gss_unwrap_data: decode databody_integ failed"));
 			return (FALSE);
 		}
 		/* Decode checksum. */
-		if (!xdr_rpc_gss_buf(xdrs, &wrapbuf, (u_int)-1)) {
+		if (!xdr_rpc_gss_buf(xdrs, &wrapbuf, RPCSEC_GSS_MAX_UNWRAP)) {
 			gss_release_buffer(&min_stat, &databuf);
 			LIBTIRPC_DEBUG(1, ("xdr_rpc_gss_unwrap_data: decode checksum failed"));
 			return (FALSE);
-- 
2.52.0