PuTTY snapshot d624ae2a (Fix double-free bug in (non-EC) Diffie-Hellman - 2018-10-08)

Source commit: a0e032cbedb41617e7e94c7eb9e37eca0b469669

source/putty/be_misc.c

@@ -91,6 +91,8 @@ void log_proxy_stderr(Plug *plug, bufchain *buf, const void *vdata, int len)
         msg = snewn(msglen+1, char);
         bufchain_fetch(buf, msg, msglen);
         bufchain_consume(buf, msglen);
+        while (msglen > 0 && (msg[msglen-1] == '\n' || msg[msglen-1] == '\r'))
+            msglen--;
         msg[msglen] = '\0';
         fullmsg = dupprintf("proxy: %s", msg);
         plug_log(plug, 2, NULL, 0, fullmsg, 0);

source/putty/errsock.c

@@ -56,11 +56,23 @@ static const SocketVtable ErrorSocket_sockvt = {
     sk_error_peer_info,
 };
 
-Socket *new_error_socket(const char *errmsg, Plug *plug)
+static Socket *new_error_socket_internal(char *errmsg, Plug *plug)
 {
     ErrorSocket *es = snew(ErrorSocket);
     es->sock.vt = &ErrorSocket_sockvt;
     es->plug = plug;
-    es->error = dupstr(errmsg);
+    es->error = errmsg;
     return &es->sock;
 }
+
+Socket *new_error_socket_fmt(Plug *plug, const char *fmt, ...)
+{
+    va_list ap;
+    char *msg;
+
+    va_start(ap, fmt);
+    msg = dupvprintf(fmt, ap);
+    va_end(ap);
+
+    return new_error_socket_internal(msg, plug);
+}

source/putty/network.h

@@ -220,7 +220,7 @@ char *get_hostname(void);
  * Trivial socket implementation which just stores an error. Found in
  * errsock.c.
  */
-Socket *new_error_socket(const char *errmsg, Plug *plug);
+Socket *new_error_socket_fmt(Plug *plug, const char *fmt, ...);
 
 /*
  * Trivial plug that does absolutely nothing. Found in nullplug.c.

source/putty/ssh.c

@@ -91,6 +91,12 @@ struct Ssh {
      */
     ConnectionLayer *cl;
 
+    /*
+     * A dummy ConnectionLayer that can be used for logging sharing
+     * downstreams that connect before the real one is ready.
+     */
+    ConnectionLayer cl_dummy;
+
     /*
      * session_started is FALSE until we initialise the main protocol
      * layers. So it distinguishes between base_layer==NULL meaning
@@ -106,6 +112,7 @@ struct Ssh {
     int need_random_unref;
 };
 
+
 #define ssh_logevent(params) ( \
         logevent_and_free((ssh)->frontend, dupprintf params))
 
@@ -640,6 +647,8 @@ static const char *connect_to_host(Ssh *ssh, const char *host, int port,
     ssh->s = ssh_connection_sharing_init(
         ssh->savedhost, ssh->savedport, ssh->conf, ssh->frontend,
         &ssh->plug, &ssh->connshare);
+    if (ssh->connshare)
+        ssh_connshare_provide_connlayer(ssh->connshare, &ssh->cl_dummy);
     ssh->attempting_connshare = FALSE;
     if (ssh->s != NULL) {
         /*
@@ -805,6 +814,7 @@ static const char *ssh_init(Frontend *frontend, Backend **backend_handle,
     *backend_handle = &ssh->backend;
 
     ssh->frontend = frontend;
+    ssh->cl_dummy.frontend = frontend;
 
     random_ref(); /* do this now - may be needed by sharing setup code */
     ssh->need_random_unref = TRUE;

source/putty/ssh2bpp.c

@@ -14,6 +14,7 @@ struct ssh2_bpp_direction {
     ssh2_cipher *cipher;
     ssh2_mac *mac;
     int etm_mode;
+    const struct ssh_compression_alg *pending_compression;
 };
 
 struct ssh2_bpp_state {
@@ -34,6 +35,7 @@ struct ssh2_bpp_state {
     ssh_compressor *out_comp;
 
     int pending_newkeys;
+    int pending_compression, seen_userauth_success;
 
     BinaryPacketProtocol bpp;
 };
@@ -90,7 +92,7 @@ void ssh2_bpp_new_outgoing_crypto(
     BinaryPacketProtocol *bpp,
     const struct ssh2_cipheralg *cipher, const void *ckey, const void *iv,
     const struct ssh2_macalg *mac, int etm_mode, const void *mac_key,
-    const struct ssh_compression_alg *compression)
+    const struct ssh_compression_alg *compression, int delayed_compression)
 {
     struct ssh2_bpp_state *s;
     assert(bpp->vt == &ssh2_bpp_vtable);
@@ -134,20 +136,31 @@ void ssh2_bpp_new_outgoing_crypto(
         s->out.mac = NULL;
     }
 
-    /* 'compression' is always non-NULL, because no compression is
-     * indicated by ssh_comp_none. But this setup call may return a
-     * null out_comp. */
-    s->out_comp = ssh_compressor_new(compression);
-    if (s->out_comp)
-        bpp_logevent(("Initialised %s compression",
-                      ssh_compressor_alg(s->out_comp)->text_name));
+    if (delayed_compression && !s->seen_userauth_success) {
+        s->out.pending_compression = compression;
+        s->out_comp = NULL;
+
+        bpp_logevent(("Will enable %s compression after user authentication",
+                      s->out.pending_compression->text_name));
+    } else {
+        s->out.pending_compression = NULL;
+
+        /* 'compression' is always non-NULL, because no compression is
+         * indicated by ssh_comp_none. But this setup call may return a
+         * null out_comp. */
+        s->out_comp = ssh_compressor_new(compression);
+
+        if (s->out_comp)
+            bpp_logevent(("Initialised %s compression",
+                          ssh_compressor_alg(s->out_comp)->text_name));
+    }
 }
 
 void ssh2_bpp_new_incoming_crypto(
     BinaryPacketProtocol *bpp,
     const struct ssh2_cipheralg *cipher, const void *ckey, const void *iv,
     const struct ssh2_macalg *mac, int etm_mode, const void *mac_key,
-    const struct ssh_compression_alg *compression)
+    const struct ssh_compression_alg *compression, int delayed_compression)
 {
     struct ssh2_bpp_state *s;
     assert(bpp->vt == &ssh2_bpp_vtable);
@@ -185,19 +198,39 @@ void ssh2_bpp_new_incoming_crypto(
         s->in.mac = NULL;
     }
 
-    /* 'compression' is always non-NULL, because no compression is
-     * indicated by ssh_comp_none. But this setup call may return a
-     * null in_decomp. */
-    s->in_decomp = ssh_decompressor_new(compression);
-    if (s->in_decomp)
-        bpp_logevent(("Initialised %s decompression",
-                      ssh_decompressor_alg(s->in_decomp)->text_name));
+    if (delayed_compression && !s->seen_userauth_success) {
+        s->in.pending_compression = compression;
+        s->in_decomp = NULL;
+
+        bpp_logevent(("Will enable %s decompression after user authentication",
+                      s->in.pending_compression->text_name));
+    } else {
+        s->in.pending_compression = NULL;
+
+        /* 'compression' is always non-NULL, because no compression is
+         * indicated by ssh_comp_none. But this setup call may return a
+         * null in_decomp. */
+        s->in_decomp = ssh_decompressor_new(compression);
+
+        if (s->in_decomp)
+            bpp_logevent(("Initialised %s decompression",
+                          ssh_decompressor_alg(s->in_decomp)->text_name));
+    }
 
     /* Clear the pending_newkeys flag, so that handle_input below will
      * start consuming the input data again. */
     s->pending_newkeys = FALSE;
 }
 
+int ssh2_bpp_rekey_inadvisable(BinaryPacketProtocol *bpp)
+{
+    struct ssh2_bpp_state *s;
+    assert(bpp->vt == &ssh2_bpp_vtable);
+    s = container_of(bpp, struct ssh2_bpp_state, bpp);
+
+    return s->pending_compression;
+}
+
 #define BPP_READ(ptr, len) do                                   \
     {                                                           \
         crMaybeWaitUntilV(s->bpp.input_eof ||                   \
@@ -207,6 +240,8 @@ void ssh2_bpp_new_incoming_crypto(
             goto eof;                                           \
     } while (0)
 
+#define userauth_range(pkttype) ((unsigned)((pkttype) - 50) < 20)
+
 static void ssh2_bpp_handle_input(BinaryPacketProtocol *bpp)
 {
     struct ssh2_bpp_state *s = container_of(bpp, struct ssh2_bpp_state, bpp);
@@ -537,6 +572,58 @@ static void ssh2_bpp_handle_input(BinaryPacketProtocol *bpp)
                  */
                 s->pending_newkeys = TRUE;
                 crWaitUntilV(!s->pending_newkeys);
+                continue;
+            }
+
+            if (type == SSH2_MSG_USERAUTH_SUCCESS) {
+                /*
+                 * Another one: if we were configured with OpenSSH's
+                 * deferred compression which is triggered on receipt
+                 * of USERAUTH_SUCCESS, then this is the moment to
+                 * turn on compression.
+                 */
+                if (s->in.pending_compression) {
+                    s->in_decomp =
+                        ssh_decompressor_new(s->in.pending_compression);
+                    bpp_logevent(("Initialised delayed %s decompression",
+                                  ssh_decompressor_alg(
+                                      s->in_decomp)->text_name));
+                    s->in.pending_compression = NULL;
+                }
+                if (s->out.pending_compression) {
+                    s->out_comp =
+                        ssh_compressor_new(s->out.pending_compression);
+                    bpp_logevent(("Initialised delayed %s compression",
+                                  ssh_compressor_alg(
+                                      s->out_comp)->text_name));
+                    s->out.pending_compression = NULL;
+                }
+
+                /*
+                 * Whether or not we were doing delayed compression in
+                 * _this_ set of crypto parameters, we should set a
+                 * flag indicating that we're now authenticated, so
+                 * that a delayed compression method enabled in any
+                 * future rekey will be treated as un-delayed.
+                 */
+                s->seen_userauth_success = TRUE;
+            }
+
+            if (s->pending_compression && userauth_range(type)) {
+                /*
+                 * Receiving any userauth message at all indicates
+                 * that we're not about to turn on delayed compression
+                 * - either because we just _have_ done, or because
+                 * this message is a USERAUTH_FAILURE or some kind of
+                 * intermediate 'please send more data' continuation
+                 * message. Either way, we turn off the outgoing
+                 * packet blockage for now, and release any queued
+                 * output packets, so that we can make another attempt
+                 * to authenticate. The next userauth packet we send
+                 * will re-block the output direction.
+                 */
+                s->pending_compression = FALSE;
+                queue_idempotent_callback(&s->bpp.ic_out_pq);
             }
         }
     }
@@ -734,6 +821,31 @@ static void ssh2_bpp_handle_output(BinaryPacketProtocol *bpp)
 {
     struct ssh2_bpp_state *s = container_of(bpp, struct ssh2_bpp_state, bpp);
     PktOut *pkt;
+    int n_userauth;
+
+    /*
+     * Count the userauth packets in the queue.
+     */
+    n_userauth = 0;
+    for (pkt = pq_first(&s->bpp.out_pq); pkt != NULL;
+         pkt = pq_next(&s->bpp.out_pq, pkt))
+        if (userauth_range(pkt->type))
+            n_userauth++;
+
+    if (s->pending_compression && !n_userauth) {
+        /*
+         * We're currently blocked from sending any outgoing packets
+         * until the other end tells us whether we're going to have to
+         * enable compression or not.
+         *
+         * If our end has pushed a userauth packet on the queue, that
+         * must mean it knows that a USERAUTH_SUCCESS is not
+         * immediately forthcoming, so we unblock ourselves and send
+         * up to and including that packet. But in this if statement,
+         * there aren't any, so we're still blocked.
+         */
+        return;
+    }
 
     if (s->cbc_ignore_workaround) {
         /*
@@ -761,7 +873,22 @@ static void ssh2_bpp_handle_output(BinaryPacketProtocol *bpp)
     }
 
     while ((pkt = pq_pop(&s->bpp.out_pq)) != NULL) {
+        if (userauth_range(pkt->type))
+            n_userauth--;
+
         ssh2_bpp_format_packet(s, pkt);
         ssh_free_pktout(pkt);
+
+        if (n_userauth == 0 && s->out.pending_compression) {
+            /*
+             * This is the last userauth packet in the queue, so
+             * unless our side decides to send another one in future,
+             * we have to assume will potentially provoke
+             * USERAUTH_SUCCESS. Block (non-userauth) outgoing packets
+             * until we see the reply.
+             */
+            s->pending_compression = TRUE;
+            return;
+        }
     }
 }

source/putty/ssh2transport.c

@@ -49,7 +49,10 @@ struct kexinit_algorithm {
             const struct ssh2_macalg *mac;
             int etm;
         } mac;
-        const struct ssh_compression_alg *comp;
+        struct {
+            const struct ssh_compression_alg *comp;
+            int delayed;
+        } comp;
     } u;
 };
 
@@ -206,6 +209,7 @@ struct ssh2_transport_state {
         const struct ssh2_macalg *mac;
         int etm_mode;
         const struct ssh_compression_alg *comp;
+        int comp_delayed;
     } in, out;
     ptrlen hostkeydata, sigdata;
     char *keystr, *fingerprint;
@@ -223,8 +227,6 @@ struct ssh2_transport_state {
     int n_preferred_ciphers;
     const struct ssh2_ciphers *preferred_ciphers[CIPHER_MAX];
     const struct ssh_compression_alg *preferred_comp;
-    int userauth_succeeded;         /* for delayed compression */
-    int pending_compression;
     int got_session_id;
     int dlgret;
     int guessok;
@@ -393,7 +395,6 @@ static void ssh2_transport_free(PacketProtocolLayer *ppl)
         ssh_key_free(s->hkey);
         s->hkey = NULL;
     }
-    if (s->e) freebn(s->e);
     if (s->f) freebn(s->f);
     if (s->p) freebn(s->p);
     if (s->g) freebn(s->g);
@@ -614,8 +615,6 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
     s->in.comp = s->out.comp = NULL;
 
     s->got_session_id = FALSE;
-    s->userauth_succeeded = FALSE;
-    s->pending_compression = FALSE;
     s->need_gss_transient_hostkey = FALSE;
     s->warned_about_no_gss_transient_hostkey = FALSE;
 
@@ -935,22 +934,23 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
             assert(lenof(compressions) > 1);
             /* Prefer non-delayed versions */
             alg = ssh2_kexinit_addalg(s->kexlists[j], s->preferred_comp->name);
-            alg->u.comp = s->preferred_comp;
-            /* We don't even list delayed versions of algorithms until
-             * they're allowed to be used, to avoid a race. See the end of
-             * this function. */
-            if (s->userauth_succeeded && s->preferred_comp->delayed_name) {
+            alg->u.comp.comp = s->preferred_comp;
+            alg->u.comp.delayed = FALSE;
+            if (s->preferred_comp->delayed_name) {
                 alg = ssh2_kexinit_addalg(s->kexlists[j],
                                           s->preferred_comp->delayed_name);
-                alg->u.comp = s->preferred_comp;
+                alg->u.comp.comp = s->preferred_comp;
+                alg->u.comp.delayed = TRUE;
             }
             for (i = 0; i < lenof(compressions); i++) {
                 const struct ssh_compression_alg *c = compressions[i];
                 alg = ssh2_kexinit_addalg(s->kexlists[j], c->name);
-                alg->u.comp = c;
-                if (s->userauth_succeeded && c->delayed_name) {
+                alg->u.comp.comp = c;
+                alg->u.comp.delayed = FALSE;
+                if (c->delayed_name) {
                     alg = ssh2_kexinit_addalg(s->kexlists[j], c->delayed_name);
-                    alg->u.comp = c;
+                    alg->u.comp.comp = c;
+                    alg->u.comp.delayed = TRUE;
                 }
             }
         }
@@ -1006,6 +1006,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
         s->in.cipher = s->out.cipher = NULL;
         s->in.mac = s->out.mac = NULL;
         s->in.comp = s->out.comp = NULL;
+        s->in.comp_delayed = s->out.comp_delayed = FALSE;
         s->warn_kex = s->warn_hk = FALSE;
         s->warn_cscipher = s->warn_sccipher = FALSE;
 
@@ -1076,21 +1077,14 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
                         s->in.mac = alg->u.mac.mac;
                         s->in.etm_mode = alg->u.mac.etm;
                     } else if (i == KEXLIST_CSCOMP) {
-                        s->out.comp = alg->u.comp;
+                        s->out.comp = alg->u.comp.comp;
+                        s->out.comp_delayed = alg->u.comp.delayed;
                     } else if (i == KEXLIST_SCCOMP) {
-                        s->in.comp = alg->u.comp;
+                        s->in.comp = alg->u.comp.comp;
+                        s->in.comp_delayed = alg->u.comp.delayed;
                     }
                     goto matched;
                 }
-
-                /* Set a flag if there's a delayed compression option
-                 * available for a compression method that we just
-                 * failed to select the immediate version of. */
-                s->pending_compression = (
-                    (i == KEXLIST_CSCOMP || i == KEXLIST_SCCOMP) &&
-                    in_commasep_string(alg->u.comp->delayed_name,
-                                       str.ptr, str.len) &&
-                    !s->userauth_succeeded);
             }
             ssh_sw_abort(s->ppl.ssh, "Couldn't agree a %s (available: %.*s)",
                          kexlist_descr[i], PTRLEN_PRINTF(str));
@@ -1127,10 +1121,6 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
             }
         }
 
-        if (s->pending_compression) {
-            ppl_logevent(("Server supports delayed compression; "
-                          "will try this later"));
-        }
         get_string(pktin);  /* client->server language */
         get_string(pktin);  /* server->client language */
         s->ignorepkt = get_bool(pktin) && !s->guessok;
@@ -1377,7 +1367,6 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
         dh_cleanup(s->dh_ctx);
         s->dh_ctx = NULL;
         freebn(s->f); s->f = NULL;
-        freebn(s->e); s->e = NULL;
         if (dh_is_gex(s->kex_alg)) {
             freebn(s->g); s->g = NULL;
             freebn(s->p); s->p = NULL;
@@ -1699,7 +1688,6 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
         dh_cleanup(s->dh_ctx);
         s->dh_ctx = NULL;
         freebn(s->f); s->f = NULL;
-        freebn(s->e); s->e = NULL;
         if (dh_is_gex(s->kex_alg)) {
             freebn(s->g); s->g = NULL;
             freebn(s->p); s->p = NULL;
@@ -2146,7 +2134,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
             s->ppl.bpp,
             s->out.cipher, cipher_key->u, cipher_iv->u,
             s->out.mac, s->out.etm_mode, mac_key->u,
-            s->out.comp);
+            s->out.comp, s->out.comp_delayed);
 
         strbuf_free(cipher_key);
         strbuf_free(cipher_iv);
@@ -2201,7 +2189,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
             s->ppl.bpp,
             s->in.cipher, cipher_key->u, cipher_iv->u,
             s->in.mac, s->in.etm_mode, mac_key->u,
-            s->in.comp);
+            s->in.comp, s->in.comp_delayed);
 
         strbuf_free(cipher_key);
         strbuf_free(cipher_iv);
@@ -2288,41 +2276,17 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
 
         if (s->rekey_class == RK_POST_USERAUTH) {
             /*
-             * userauth has seen a USERAUTH_SUCCEEDED. For a couple of
-             * reasons, this may be the moment to do an immediate
-             * rekey with different parameters. But it may not; so
-             * here we turn that rekey class into either RK_NONE or
-             * RK_NORMAL.
+             * userauth has seen a USERAUTH_SUCCESS. This may be the
+             * moment to do an immediate rekey with different
+             * parameters. But it may not; so here we turn that rekey
+             * class into either RK_NONE or RK_NORMAL.
              *
-             * One is to turn on delayed compression. We do this by a
-             * rekey to work around a protocol design bug:
-             * draft-miller-secsh-compression-delayed-00 says that you
-             * negotiate delayed compression in the first key
-             * exchange, and both sides start compressing when the
-             * server has sent USERAUTH_SUCCESS. This has a race
-             * condition -- the server can't know when the client has
-             * seen it, and thus which incoming packets it should
-             * treat as compressed.
-             *
-             * Instead, we do the initial key exchange without
-             * offering the delayed methods, but note if the server
-             * offers them; when we get here, if a delayed method was
-             * available that was higher on our list than what we got,
-             * we initiate a rekey in which we _do_ list the delayed
-             * methods (and hopefully get it as a result). Subsequent
-             * rekeys will do the same.
-             *
-             * Another reason for a rekey at this point is if we've
-             * done a GSS key exchange and don't have anything in our
+             * Currently the only reason for this is if we've done a
+             * GSS key exchange and don't have anything in our
              * transient hostkey cache, in which case we should make
              * an attempt to populate the cache now.
              */
-            assert(!s->userauth_succeeded); /* should only happen once */
-            s->userauth_succeeded = TRUE;
-            if (s->pending_compression) {
-                s->rekey_reason = "enabling delayed compression";
-                s->rekey_class = RK_NORMAL;
-            } else if (s->need_gss_transient_hostkey) {
+            if (s->need_gss_transient_hostkey) {
                 s->rekey_reason = "populating transient host key cache";
                 s->rekey_class = RK_NORMAL;
             } else {
@@ -2908,7 +2872,7 @@ static void ssh2_transport_reconfigure(PacketProtocolLayer *ppl, Conf *conf)
     s->conf = conf_copy(conf);
 
     if (rekey_reason) {
-        if (!s->kex_in_progress) {
+        if (!s->kex_in_progress && !ssh2_bpp_rekey_inadvisable(s->ppl.bpp)) {
             s->rekey_reason = rekey_reason;
             s->rekey_class = RK_NORMAL;
             queue_idempotent_callback(&s->ppl.ic_process_queue);

source/putty/sshbpp.h

@@ -103,12 +103,23 @@ void ssh2_bpp_new_outgoing_crypto(
     BinaryPacketProtocol *bpp,
     const struct ssh2_cipheralg *cipher, const void *ckey, const void *iv,
     const struct ssh2_macalg *mac, int etm_mode, const void *mac_key,
-    const struct ssh_compression_alg *compression);
+    const struct ssh_compression_alg *compression, int delayed_compression);
 void ssh2_bpp_new_incoming_crypto(
     BinaryPacketProtocol *bpp,
     const struct ssh2_cipheralg *cipher, const void *ckey, const void *iv,
     const struct ssh2_macalg *mac, int etm_mode, const void *mac_key,
-    const struct ssh_compression_alg *compression);
+    const struct ssh_compression_alg *compression, int delayed_compression);
+
+/*
+ * A query method specific to the interface between ssh2transport and
+ * ssh2bpp. If true, it indicates that we're potentially in the
+ * race-condition-prone part of delayed compression setup and so
+ * asynchronous outgoing transport-layer packets are currently not
+ * being sent, which means in particular that it would be a bad idea
+ * to start a rekey because then we'd stop responding to anything
+ * _other_ than transport-layer packets and deadlock the protocol.
+ */
+int ssh2_bpp_rekey_inadvisable(BinaryPacketProtocol *bpp);
 
 BinaryPacketProtocol *ssh2_bare_bpp_new(Frontend *frontend);
 

source/putty/sshverstring.c

@@ -409,8 +409,10 @@ static PktOut *ssh_verstring_new_pktout(int type)
 
 static void ssh_verstring_handle_output(BinaryPacketProtocol *bpp)
 {
-    assert(0 && "Should never try to send packets during SSH version "
-           "string exchange");
+    if (pq_peek(&bpp->out_pq)) {
+        assert(0 && "Should never try to send packets during SSH version "
+               "string exchange");
+    }
 }
 
 /*

source/putty/windows/winproxy.c

@@ -50,30 +50,30 @@ Socket *platform_new_connection(SockAddr *addr, const char *hostname,
     sa.lpSecurityDescriptor = NULL;    /* default */
     sa.bInheritHandle = TRUE;
     if (!CreatePipe(&us_from_cmd, &cmd_to_us, &sa, 0)) {
-	Socket *ret =
-            new_error_socket("Unable to create pipes for proxy command", plug);
         sfree(cmd);
-	return ret;
+	return new_error_socket_fmt(
+            plug, "Unable to create pipes for proxy command: %s",
+            win_strerror(GetLastError()));
     }
 
     if (!CreatePipe(&cmd_from_us, &us_to_cmd, &sa, 0)) {
-	Socket *ret =
-            new_error_socket("Unable to create pipes for proxy command", plug);
         sfree(cmd);
 	CloseHandle(us_from_cmd);
 	CloseHandle(cmd_to_us);
-	return ret;
+	return new_error_socket_fmt(
+            plug, "Unable to create pipes for proxy command: %s",
+            win_strerror(GetLastError()));
     }
 
     if (!CreatePipe(&us_from_cmd_err, &cmd_err_to_us, &sa, 0)) {
-        Socket *ret = new_error_socket
-            ("Unable to create pipes for proxy command", plug);
         sfree(cmd);
         CloseHandle(us_from_cmd);
         CloseHandle(cmd_to_us);
         CloseHandle(us_to_cmd);
         CloseHandle(cmd_from_us);
-        return ret;
+        return new_error_socket_fmt(
+            plug, "Unable to create pipes for proxy command: %s",
+            win_strerror(GetLastError()));
     }
 
     SetHandleInformation(us_to_cmd, HANDLE_FLAG_INHERIT, 0);