[freenet-dev] [freenet-cvs] r15120 - trunk/freenet/src/freenet/node/fcp

[Florent Daignière]

* Matthew Toseland <toad at amphibian.dyndns.org> [2007-09-10 22:50:43]:

> I don't see how this changes anything: ConnectionInputHandler already checks 
> for whether the connection has been closed:
> 
> 			if(handler.isClosed()) return;
> 
> I've committed a better fix.

Sure, I chose to close the socket directly... whereas you close the
input stream in r15121 ... the old code wasn't doing neither anyway and
that was the "bug".

> 
> On Monday 10 September 2007 20:54, you wrote:
> > Author: nextgens
> > Date: 2007-09-10 19:54:34 +0000 (Mon, 10 Sep 2007)
> > New Revision: 15120
> > 
> > Modified:
> >    trunk/freenet/src/freenet/node/fcp/FCPConnectionHandler.java
> >    trunk/freenet/src/freenet/node/fcp/FCPConnectionInputHandler.java
> > Log:
> > Fix a potential security issue with FCP -thanks to makomk for the heads up-
> > Will detail the vulnerability when this fix hits stable.
> > 
> > Modified: trunk/freenet/src/freenet/node/fcp/FCPConnectionHandler.java
> > ===================================================================
> > --- trunk/freenet/src/freenet/node/fcp/FCPConnectionHandler.java	2007-09-10 
> 19:24:45 UTC (rev 15119)
> > +++ trunk/freenet/src/freenet/node/fcp/FCPConnectionHandler.java	2007-09-10 
> 19:54:34 UTC (rev 15120)
> > @@ -97,6 +97,7 @@
> >  			requests = new ClientRequest[requestsByIdentifier.size()];
> >  			requests = (ClientRequest[]) 
> requestsByIdentifier.values().toArray(requests);
> >  		}
> > +		try { sock.close(); } catch (IOException e) {}
> >  		for(int i=0;i<requests.length;i++)
> >  			requests[i].onLostConnection();
> >  		if((client != null) && !client.hasPersistentRequests())
> > 
> > Modified: trunk/freenet/src/freenet/node/fcp/FCPConnectionInputHandler.java
> > ===================================================================
> > --- trunk/freenet/src/freenet/node/fcp/FCPConnectionInputHandler.java	
> 2007-09-10 19:24:45 UTC (rev 15119)
> > +++ trunk/freenet/src/freenet/node/fcp/FCPConnectionInputHandler.java	
> 2007-09-10 19:54:34 UTC (rev 15120)
> > @@ -67,11 +67,12 @@
> >  				is.close();
> >  				return;
> >  			}
> > -			if(messageType.equals("")) continue;
> > +			if(messageType.equals(""))
> > +				continue;
> >  			fs = new SimpleFieldSet(lis, 4096, 128, true, true, true, true);
> >  			
> >  			// check for valid endmarker
> > -			if (fs.getEndMarker() != null && (!fs.getEndMarker().startsWith("End")) 
> && (!"Data".equals(fs.getEndMarker()))) {
> > +			if (!firstMessage && fs.getEndMarker() != null && 
> (!fs.getEndMarker().startsWith("End")) && 
> (!"Data".equals(fs.getEndMarker()))) {
> >  				FCPMessage err = new 
> ProtocolErrorMessage(ProtocolErrorMessage.MESSAGE_PARSE_ERROR, 
> false, "Invalid end marker: "+fs.getEndMarker(), fs.get("Identifer"), 
> fs.getBoolean("Global", false));
> >  				handler.outputHandler.queue(err);
> >  				continue;
> > @@ -84,8 +85,15 @@
> >  				msg = FCPMessage.create(messageType, fs, handler.bf, 
> handler.server.core.persistentTempBucketFactory);
> >  				if(msg == null) continue;
> >  			} catch (MessageInvalidException e) {
> > -				FCPMessage err = new ProtocolErrorMessage(e.protocolCode, false, 
> e.getMessage(), e.ident, e.global);
> > -				handler.outputHandler.queue(err);
> > +				if(firstMessage) {
> > +					FCPMessage err = new 
> ProtocolErrorMessage(ProtocolErrorMessage.CLIENT_HELLO_MUST_BE_FIRST_MESSAGE, 
> true, null, null, false);
> > +					handler.outputHandler.queue(err);
> > +					handler.close();
> > +					continue;
> > +				} else {
> > +					FCPMessage err = new ProtocolErrorMessage(e.protocolCode, false, 
> e.getMessage(), e.ident, e.global);
> > +					handler.outputHandler.queue(err);
> > +				}
> >  				continue;
> >  			}
> >  			if(firstMessage && !(msg instanceof ClientHelloMessage)) {
> > 
> > _______________________________________________
> > cvs mailing list
> > cvs at freenetproject.org
> > http://emu.freenetproject.org/cgi-bin/mailman/listinfo/cvs
> > 
> > 



> _______________________________________________
> Devl mailing list
> Devl at freenetproject.org
> http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://emu.freenetproject.org/pipermail/devl/attachments/20070911/c762834d/attachment.pgp