[src/trunk]: src/crypto/external/bsd/openssh/dist Coverity issues 996120 and ...

[spz <spz%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/6600554e55f2
branches:  trunk
changeset: 792000:6600554e55f2
user:      spz <spz%NetBSD.org@localhost>
date:      Sun Dec 15 10:42:52 2013 +0000

description:
Coverity issues 996120 and 996121, Use after free

Use the M_CP_STROPT definition exclusive to servconf.c twice and
you have freed your original string.

servconf.h won copying authorized_keys_command and
authorized_keys_command_user in COPY_MATCH_STRING_OPTS in 1.107,
but servconf.c didn't drop its own, so it walks into this trap.
Remove the duplicate copies, and disarm the trap.

Note this is on a code path where authorized_keys_command and
authorized_keys_command_user don't actually get used except
for a debug dump of the config, and dump_cfg_string protects
itself against trying to print NULL pointers, so all
you get is sshd -T -C ... giving wrong results, which is rather
insignificant as far as security issues go.

diffstat:

 crypto/external/bsd/openssh/dist/servconf.c |  8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diffs (36 lines):

diff -r aa5932a97313 -r 6600554e55f2 crypto/external/bsd/openssh/dist/servconf.c
--- a/crypto/external/bsd/openssh/dist/servconf.c       Sun Dec 15 10:25:23 2013 +0000
+++ b/crypto/external/bsd/openssh/dist/servconf.c       Sun Dec 15 10:42:52 2013 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: servconf.c,v 1.13 2013/11/08 19:18:25 christos Exp $   */
+/*     $NetBSD: servconf.c,v 1.14 2013/12/15 10:42:52 spz Exp $        */
 /* $OpenBSD: servconf.c,v 1.240 2013/07/19 07:37:48 markus Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo%cs.hut.fi@localhost>, Espoo, Finland
@@ -12,7 +12,7 @@
  */
 
 #include "includes.h"
-__RCSID("$NetBSD: servconf.c,v 1.13 2013/11/08 19:18:25 christos Exp $");
+__RCSID("$NetBSD: servconf.c,v 1.14 2013/12/15 10:42:52 spz Exp $");
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/queue.h>
@@ -2012,7 +2012,7 @@
                dst->n = src->n; \
 } while (0)
 #define M_CP_STROPT(n) do {\
-       if (src->n != NULL) { \
+       if (src->n != NULL && dst->n != src->n) { \
                free(dst->n); \
                dst->n = src->n; \
        } \
@@ -2043,8 +2043,6 @@
        M_CP_INTOPT(hostbased_uses_name_from_packet_only);
        M_CP_INTOPT(kbd_interactive_authentication);
        M_CP_INTOPT(zero_knowledge_password_authentication);
-       M_CP_STROPT(authorized_keys_command);
-       M_CP_STROPT(authorized_keys_command_user);
        M_CP_INTOPT(permit_root_login);
        M_CP_INTOPT(permit_empty_passwd);