From baa667c7c433c0136c7b7eac3a26b7fe802a7613 Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Fri, 16 Nov 2018 06:10:30 +0000 Subject: make grandparent-parent-child sshbuf chains robust to use-after-free faults if the ancestors are freed before the descendents. Nothing in OpenSSH uses this deallocation pattern. Reported by Jann Horn --- usr.bin/ssh/sshbuf.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/usr.bin/ssh/sshbuf.c b/usr.bin/ssh/sshbuf.c index 90990c310dc..f071c1848af 100644 --- a/usr.bin/ssh/sshbuf.c +++ b/usr.bin/ssh/sshbuf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshbuf.c,v 1.12 2018/07/09 21:56:06 markus Exp $ */ +/* $OpenBSD: sshbuf.c,v 1.13 2018/11/16 06:10:29 djm Exp $ */ /* * Copyright (c) 2011 Damien Miller * @@ -141,12 +141,7 @@ sshbuf_free(struct sshbuf *buf) */ if (sshbuf_check_sanity(buf) != 0) return; - /* - * If we are a child, the free our parent to decrement its reference - * count and possibly free it. - */ - sshbuf_free(buf->parent); - buf->parent = NULL; + /* * If we are a parent with still-extant children, then don't free just * yet. The last child's call to sshbuf_free should decrement our @@ -155,6 +150,14 @@ sshbuf_free(struct sshbuf *buf) buf->refcount--; if (buf->refcount > 0) return; + + /* + * If we are a child, the free our parent to decrement its reference + * count and possibly free it. + */ + sshbuf_free(buf->parent); + buf->parent = NULL; + if (!buf->readonly) { explicit_bzero(buf->d, buf->alloc); free(buf->d); -- cgit v1.2.3