On Monday 14 April 2008 06:28, nextgens at freenetproject.org wrote:
> Author: nextgens
> Date: 2008-04-14 05:28:57 +0000 (Mon, 14 Apr 2008)
> New Revision: 19303
>
> Modified:
> trunk/freenet/src/freenet/keys/ClientCHKBlock.java
> trunk/freenet/src/freenet/node/FNPPacketMangler.java
> trunk/freenet/src/freenet/node/LocationManager.java
> Log:
> fix two potential security weaknesses (Would thow ArrayOutOfBounds)
>
> Modified: trunk/freenet/src/freenet/keys/ClientCHKBlock.java
> ===================================================================
> --- trunk/freenet/src/freenet/keys/ClientCHKBlock.java 2008-04-14
> 04:29:38
UTC (rev 19302)
> +++ trunk/freenet/src/freenet/keys/ClientCHKBlock.java 2008-04-14
> 05:28:57
UTC (rev 19303)
> @@ -89,8 +89,8 @@
> throw new CHKDecodeException("Crypto key too short");
> cipher.initialize(key.cryptoKey);
> PCFBMode pcfb = PCFBMode.create(cipher);
> - byte[] hbuf = new byte[headers.length-2];
> - System.arraycopy(headers, 2, hbuf, 0, headers.length-2);
> + byte[] hbuf = new byte[headers.length > 2 ? headers.length-2 : 0];
> + System.arraycopy(headers, 2, hbuf, 0, hbuf.length);
I disagree. We should throw. And we do, in the CHKBlock constructor. Please
revert.
> byte[] dbuf = new byte[data.length];
> System.arraycopy(data, 0, dbuf, 0, data.length);
> // Decipher header first - functions as IV
>
> Modified: trunk/freenet/src/freenet/node/FNPPacketMangler.java
> ===================================================================
> --- trunk/freenet/src/freenet/node/FNPPacketMangler.java 2008-04-14
> 04:29:38
UTC (rev 19302)
> +++ trunk/freenet/src/freenet/node/FNPPacketMangler.java 2008-04-14
> 05:28:57
UTC (rev 19303)
> @@ -1124,7 +1124,7 @@
> byte[] data = new byte[decypheredPayload.length -
decypheredPayloadOffset];
> System.arraycopy(decypheredPayload, decypheredPayloadOffset,
> data, 0,
decypheredPayload.length - decypheredPayloadOffset);
> long bootID = Fields.bytesToLong(data);
> - byte[] hisRef = new byte[data.length -8];
> + byte[] hisRef = new byte[data.length > 8 ? data.length -8 : 0];
Here we definitely shouldn't swallow it silently, we should log it and kill
the packet, as we do with other formatting errors. We should not allocate a
zero length array and then have devs running in circles trying to figure out
what's going on!
> System.arraycopy(data, 8, hisRef, 0, hisRef.length);
>
> // construct the peernode
>
> Modified: trunk/freenet/src/freenet/node/LocationManager.java
> ===================================================================
> --- trunk/freenet/src/freenet/node/LocationManager.java 2008-04-14
> 04:29:38
UTC (rev 19302)
> +++ trunk/freenet/src/freenet/node/LocationManager.java 2008-04-14
> 05:28:57
UTC (rev 19303)
> @@ -349,7 +349,7 @@
> }
> registerKnownLocation(hisLoc);
>
> - double[] hisFriendLocs = new double[hisBufLong.length-2];
> + double[] hisFriendLocs = new double[hisBufLong.length > 2 ?
hisBufLong.length-2 : 0];
> for(int i=0;i<hisFriendLocs.length;i++) {
> hisFriendLocs[i] =
Double.longBitsToDouble(hisBufLong[i+2]);
> if((hisFriendLocs[i] < 0.0) || (hisFriendLocs[i] > 1.0)) {
> @@ -546,7 +546,7 @@
> }
> registerKnownLocation(hisLoc);
>
> - double[] hisFriendLocs = new double[hisBufLong.length-2];
> + double[] hisFriendLocs = new double[hisBufLong.length > 2 ?
hisBufLong.length-2 : 0];
Likewise here, we should do proper error handling. It's not that hard.
> for(int i=0;i<hisFriendLocs.length;i++) {
> hisFriendLocs[i] =
Double.longBitsToDouble(hisBufLong[i+2]);
> if((hisFriendLocs[i] < 0.0) || (hisFriendLocs[i] >
1.0)) {
>
> _______________________________________________
> 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:
<https://emu.freenetproject.org/pipermail/devl/attachments/20080414/cee9d96d/attachment.pgp>
