Date: Mon, 24 Nov 1997 03:36:41 -0800 From: Don Lewis <Don.Lewis@tsc.tdk.com> To: Don Lewis <Don.Lewis@tsc.tdk.com>, Jim Shankland <jas@flyingfox.com>, robert@cyrus.watson.org Cc: security@freebsd.org Subject: Re: new TCP/IP bug in win95 (fwd) Message-ID: <199711241136.DAA21524@salsa.gv.tsc.tdk.com> In-Reply-To: Don Lewis <Don.Lewis@tsc.tdk.com> "Re: new TCP/IP bug in win95 (fwd)" (Nov 21, 4:37pm)
next in thread | previous in thread | raw e-mail | index | archive | help
On Nov 21, 4:37pm, Don Lewis wrote: } Subject: Re: new TCP/IP bug in win95 (fwd) } --- tcp_input.c.prev Fri Nov 21 04:34:51 1997 } +++ tcp_input.c Fri Nov 21 16:32:10 1997 } @@ -752,6 +752,18 @@ } } } } /* } + * If the state is SYN_RCVD: } + * If seg contains a SYN,ACK, then drop it and send a RST. } + * We should only ever get an ACK or a duplicate SYN (if our } + * SYN,ACK was lost) in this state. } + * Otherwise continue processing } + */ } + case TCPS_SYN_RECEIVED: } + if ((tiflags & (TH_SYN|TH_ACK)) == (TH_SYN|TH_ACK)) } + goto dropwithreset; } + break; /* continue normal processing */ } + } + /* } * If the state is SYN_SENT: } * if seg contains an ACK, but not for our SYN, drop the input. } * if seg contains a RST, then drop the connection. }-- End of excerpt from Don Lewis I'm pretty sure this breaks simultaneous opens and self-connects, so how about the following? --- tcp_input.c.prev Fri Nov 21 04:34:51 1997 +++ tcp_input.c Mon Nov 24 03:12:11 1997 @@ -752,6 +752,32 @@ } /* + * If the state is SYN_RCVD: + * If the segment contains a SYN and the sequence number + * doesn't match the initial receive sequence number which + * was set by the previous SYN, drop the segment and send + * a RST. + * + * We'd also like to drop the segment and send a RST if + * the segment contains SYN-ACK, but we'll receive this + * in the (uncommon) simultaneous open or self-connect + * cases. In the usual case, we should only ever get an + * ACK or a duplicate SYN (if our SYN-ACK was lost) in + * this state. It would be ideal if we could perform this + * additional check if the previous state was LISTEN and + * skip this check if the previous state was SYN_SENT. + * As it stands, it's possible for a forged SYN to cause + * us to do a self-connect on a listening socket if the + * proper sequence number can be guessed. + * + * Otherwise continue processing + */ + case TCPS_SYN_RECEIVED: + if ((tiflags & TH_SYN) && ti->seq != tp->irs) + goto dropwithreset; + break; /* continue normal processing */ + + /* * If the state is SYN_SENT: * if seg contains an ACK, but not for our SYN, drop the input. * if seg contains a RST, then drop the connection. BTW, does anyone else think that instead of "goto dropwithreset" that this should be a call to tcp_drop()? If we tell our client to go away, it would seem there's no sense in keeping our socket around until it times out, though I suppose it will go away when we retry the SYN-ACK and get a RST. I might also be convinced that this should just be "goto drop". Likewise in the code below: /* * Ack processing. */ switch (tp->t_state) { /* * In SYN_RECEIVED state if the ack ACKs our SYN then enter * ESTABLISHED state and continue processing, otherwise * send an RST. */ case TCPS_SYN_RECEIVED: if (SEQ_GT(tp->snd_una, ti->ti_ack) || SEQ_GT(ti->ti_ack, tp->snd_max)) goto dropwithreset; Also the following looks wrong to me. Doesn't it end up sending two RST packets? /* * If a SYN is in the window, then this is an * error and we send an RST and drop the connection. */ if (tiflags & TH_SYN) { tp = tcp_drop(tp, ECONNRESET); goto dropwithreset; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199711241136.DAA21524>