upstream: 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

OpenBSD-Commit-ID: d93501d1d2734245aac802a252b9bb2eccdba0f2

diff --git a/sshbuf.c b/sshbuf.c
index 20ddf9eb62725cd11bd8085b6a325517d9038b09..adfddf7758c1b8b72dd39f0697b2f99e60b3097d 100644 (file)
--- a/sshbuf.c
+++ b/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
  *
@@ -143,12 +143,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
@@ -157,6 +152,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);