Index | Thread | Search

From:
Jonathan Matthew <jonathan@d14n.org>
Subject:
ypldap: tighten up parsing of paged search controls
To:
tech@openbsd.org
Cc:
jan@openbsd.org, martijn@openbsd.org
Date:
Fri, 3 Jul 2026 17:10:54 +1000

Download raw body.

Thread
  • Jonathan Matthew:

    ypldap: tighten up parsing of paged search controls

Several of the swival reports related to the code in aldap.c for
parsing the paging control in ldap search results, which is overly
trusting that the response from the server will be properly formed.

I hacked up an ldap proxy to make it send bad paging controls back to
the client and that pointed out a few more checks we should add in
addition to what the swival reports suggested.  The same diff can also
be applied to the copies of aldap.c in libexec/login_ldap/ and
usr.bin/ldap/.

If it can't parse the paging control, ypldap will stop the search
there, so it'll only have the first page of results.

ok?


Index: aldap.c
===================================================================
RCS file: /cvs/src/usr.sbin/ypldap/aldap.c,v
diff -u -p -r1.50 aldap.c
--- aldap.c	30 Jun 2026 18:20:28 -0000	1.50
+++ aldap.c	3 Jul 2026 07:01:49 -0000
@@ -387,7 +387,7 @@ aldap_parse(struct aldap *ldap)
 	unsigned int		 type;
 	long long		 msgid = 0;
 	struct aldap_message	*m;
-	struct ber_element	*a = NULL, *ep;
+	struct ber_element	*a = NULL, *ep, *ctl;
 	char			 rbuf[512];
 	int			 ret, retry;
 
@@ -454,9 +454,15 @@ aldap_parse(struct aldap *ldap)
 		if (m->msg->be_sub) {
 			for (ep = m->msg->be_sub; ep != NULL; ep = ep->be_next) {
 				ober_scanf_elements(ep, "t", &class, &type);
-				if (class == 2 && type == 0)
-					m->page = aldap_parse_page_control(ep->be_sub->be_sub,
-					    ep->be_sub->be_sub->be_len);
+				if (class == 2 && type == 0) {
+					if (ep->be_sub == NULL ||
+					    ep->be_sub->be_sub == NULL)
+						goto parsefail;
+
+					ctl = ep->be_sub->be_sub;
+					m->page = aldap_parse_page_control(ctl,
+					    ctl->be_len);
+				}
 			}
 		} else
 			m->page = NULL;
@@ -492,36 +498,39 @@ aldap_parse_page_control(struct ber_elem
 	char *oid, *s;
 	char *encoded;
 	struct ber b;
-	struct ber_element *elm;
-	struct aldap_page_control *page;
+	struct ber_element *elm = NULL;
+	struct aldap_page_control *page = NULL;
 
 	b.br_wbuf = NULL;
-	ober_scanf_elements(control, "ss", &oid, &encoded);
+	if (ober_scanf_elements(control, "ss", &oid, &encoded) == -1)
+		goto failed;
+
 	ober_set_readbuf(&b, encoded, control->be_next->be_len);
 	elm = ober_read_elements(&b, NULL);
+	if (elm == NULL)
+		goto failed;
 
-	if ((page = malloc(sizeof(struct aldap_page_control))) == NULL) {
-		if (elm != NULL)
-			ober_free_elements(elm);
-		ober_free(&b);
-		return NULL;
-	}
+	if ((page = malloc(sizeof(struct aldap_page_control))) == NULL)
+		goto failed;
+
+	if (ober_scanf_elements(elm->be_sub, "is", &page->size, &s) == -1)
+		goto failed;
 
-	ober_scanf_elements(elm->be_sub, "is", &page->size, &s);
 	page->cookie_len = elm->be_sub->be_next->be_len;
+	if ((page->cookie = malloc(page->cookie_len)) == NULL)
+		goto failed;
 
-	if ((page->cookie = malloc(page->cookie_len)) == NULL) {
-		if (elm != NULL)
-			ober_free_elements(elm);
-		ober_free(&b);
-		free(page);
-		return NULL;
-	}
 	memcpy(page->cookie, s, page->cookie_len);
 
 	ober_free_elements(elm);
 	ober_free(&b);
 	return page;
+ failed:
+	LDAP_DEBUG("couldn't parse paging control", control);
+	ober_free_elements(elm);
+	ober_free(&b);
+	free(page);
+	return NULL;
 }
 
 void