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

[Matthew Toseland]

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.

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
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://emu.freenetproject.org/pipermail/devl/attachments/20070910/280cc060/attachment.pgp