Merge pull request #1057 from DaanHoogland/CWE-190

Cwe 190coverity warnings of this type adressed. Some where dismissed and maybe with reason but it seemed possible to remove them and hence obligatory ;p

* pr/1057:
  move back to original contract of isNetworksOverlap()
  Changed the behavior of methods that use NetUtils.cidrToLong(String)
  CWE-190 unit test for extremes of long netMaskFromCidr(long)
  CWE-190 netmask as long form cidr-size as method
  CID-1116482 cidrToLong cleanup of bitshift problem
  CID-1116483 cidr to netmask bitshifts guarded with casts
  CID-1116484 cast to long and use long as cidrsize type  and simpel test
  CID-1116485: cast cidr during bit shifting  and simple test included
  CID-1175714 casts before bit shift

Signed-off-by: Daan Hoogland <daan@onecht.net>
This commit is contained in:
Daan Hoogland 2015-12-06 20:10:56 +01:00
commit afe1130920
3 changed files with 154 additions and 37 deletions

View File

@ -418,7 +418,10 @@ public class ByteBuffer {
}
protected static long calculateUnsignedInt(byte value1, byte value2, byte value3, byte value4) {
return (calculateUnsignedByte(value1) << 24) + (calculateUnsignedByte(value2) << 16) + (calculateUnsignedByte(value3) << 8) + calculateUnsignedByte(value4);
return (((long)calculateUnsignedByte(value1)) << 24)
+ (((long)calculateUnsignedByte(value2)) << 16)
+ (((long)calculateUnsignedByte(value3)) << 8)
+ calculateUnsignedByte(value4);
}
/**

View File

@ -40,6 +40,7 @@ import java.util.TreeSet;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.SystemUtils;
import org.apache.commons.net.util.SubnetUtils;
import org.apache.commons.validator.routines.InetAddressValidator;
@ -47,6 +48,7 @@ import org.apache.log4j.Logger;
import com.cloud.utils.IteratorUtil;
import com.cloud.utils.Pair;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.script.Script;
import com.googlecode.ipv6.IPv6Address;
import com.googlecode.ipv6.IPv6AddressRange;
@ -775,11 +777,19 @@ public class NetUtils {
}
public static String getCidrSubNet(final String ip, final long cidrSize) {
final long numericNetmask = 0xffffffff >> MAX_CIDR - cidrSize << MAX_CIDR - cidrSize;
final long numericNetmask = netMaskFromCidr(cidrSize);
final String netmask = NetUtils.long2Ip(numericNetmask);
return getSubNet(ip, netmask);
}
/**
* @param cidrSize
* @return
*/
static long netMaskFromCidr(final long cidrSize) {
return ((long)0xffffffff) >> MAX_CIDR - cidrSize << MAX_CIDR - cidrSize;
}
public static String ipAndNetMaskToCidr(final String ip, final String netmask) {
if (!isValidIp(ip)) {
return null;
@ -829,13 +839,13 @@ public class NetUtils {
}
public static SupersetOrSubset isNetowrkASubsetOrSupersetOfNetworkB(final String cidrA, final String cidrB) {
if (!areCidrsNotEmpty(cidrA, cidrB)) {
return SupersetOrSubset.errorInCidrFormat;
}
final Long[] cidrALong = cidrToLong(cidrA);
final Long[] cidrBLong = cidrToLong(cidrB);
long shift = 0;
if (cidrALong == null || cidrBLong == null) {
//implies error in the cidr format
return SupersetOrSubset.errorInCidrFormat;
}
if (cidrALong[1] >= cidrBLong[1]) {
shift = MAX_CIDR - cidrBLong[1];
} else {
@ -858,40 +868,58 @@ public class NetUtils {
}
public static boolean isNetworkAWithinNetworkB(final String cidrA, final String cidrB) {
final Long[] cidrALong = cidrToLong(cidrA);
final Long[] cidrBLong = cidrToLong(cidrB);
if (cidrALong == null || cidrBLong == null) {
if (!areCidrsNotEmpty(cidrA, cidrB)) {
return false;
}
final long shift = MAX_CIDR - cidrBLong[1];
Long[] cidrALong = cidrToLong(cidrA);
Long[] cidrBLong = cidrToLong(cidrB);
long shift = MAX_CIDR - cidrBLong[1];
return cidrALong[0] >> shift == cidrBLong[0] >> shift;
}
static boolean areCidrsNotEmpty(String cidrA, String cidrB) {
return StringUtils.isNotEmpty(cidrA) && StringUtils.isNotEmpty(cidrB);
}
public static Long[] cidrToLong(final String cidr) {
if (cidr == null || cidr.isEmpty()) {
return null;
throw new CloudRuntimeException("empty cidr can not be converted to longs");
}
final String[] cidrPair = cidr.split("\\/");
if (cidrPair.length != 2) {
return null;
throw new CloudRuntimeException("cidr is not formatted correctly: "+ cidr);
}
final String cidrAddress = cidrPair[0];
final String cidrSize = cidrPair[1];
if (!isValidIp(cidrAddress)) {
return null;
throw new CloudRuntimeException("cidr is not valid in ip space" + cidr);
}
int cidrSizeNum = -1;
long cidrSizeNum = getCidrSizeFromString(cidrSize);
final long numericNetmask = netMaskFromCidr(cidrSizeNum);
final long ipAddr = ip2Long(cidrAddress);
final Long[] cidrlong = {ipAddr & numericNetmask, cidrSizeNum};
return cidrlong;
}
/**
* @param cidrSize
* @return
* @throws CloudRuntimeException
*/
static long getCidrSizeFromString(final String cidrSize) throws CloudRuntimeException {
long cidrSizeNum = -1;
try {
cidrSizeNum = Integer.parseInt(cidrSize);
} catch (final Exception e) {
return null;
} catch (final NumberFormatException e) {
throw new CloudRuntimeException("cidrsize is not a valid int: " + cidrSize, e);
}
final long numericNetmask = 0xffffffff >> MAX_CIDR - cidrSizeNum << MAX_CIDR - cidrSizeNum;
final long ipAddr = ip2Long(cidrAddress);
final Long[] cidrlong = {ipAddr & numericNetmask, (long)cidrSizeNum};
return cidrlong;
if(cidrSizeNum > 32 || cidrSizeNum < 0) {// assuming IPv4
throw new CloudRuntimeException("cidr size out of range: " + cidrSizeNum);
}
return cidrSizeNum;
}
public static String getCidrSubNet(final String cidr) {
@ -907,20 +935,14 @@ public class NetUtils {
if (!isValidIp(cidrAddress)) {
return null;
}
int cidrSizeNum = -1;
try {
cidrSizeNum = Integer.parseInt(cidrSize);
} catch (final Exception e) {
return null;
}
final long numericNetmask = 0xffffffff >> MAX_CIDR - cidrSizeNum << MAX_CIDR - cidrSizeNum;
long cidrSizeNum = getCidrSizeFromString(cidrSize);
final long numericNetmask = netMaskFromCidr(cidrSizeNum);
final String netmask = NetUtils.long2Ip(numericNetmask);
return getSubNet(cidrAddress, netmask);
}
public static String getCidrNetmask(final long cidrSize) {
final long numericNetmask = 0xffffffff >> MAX_CIDR - cidrSize << MAX_CIDR - cidrSize;
final long numericNetmask = netMaskFromCidr(cidrSize);
return long2Ip(numericNetmask);
}
@ -1148,13 +1170,15 @@ public class NetUtils {
}
public static boolean isNetworksOverlap(final String cidrA, final String cidrB) {
final Long[] cidrALong = cidrToLong(cidrA);
final Long[] cidrBLong = cidrToLong(cidrB);
if (cidrALong == null || cidrBLong == null) {
return false;
try {
Long[] cidrALong = cidrToLong(cidrA);
Long[] cidrBLong = cidrToLong(cidrB);
final long shift = MAX_CIDR - (cidrALong[1] > cidrBLong[1] ? cidrBLong[1] : cidrALong[1]);
return cidrALong[0] >> shift == cidrBLong[0] >> shift;
} catch (CloudRuntimeException e) {
s_logger.error(e.getLocalizedMessage(),e);
}
final long shift = MAX_CIDR - (cidrALong[1] > cidrBLong[1] ? cidrBLong[1] : cidrALong[1]);
return cidrALong[0] >> shift == cidrBLong[0] >> shift;
return false;
}
public static boolean isValidS2SVpnPolicy(final String policys) {

View File

@ -38,6 +38,8 @@ import java.util.TreeSet;
import org.apache.log4j.Logger;
import org.junit.Test;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.net.NetUtils.SupersetOrSubset;
import com.googlecode.ipv6.IPv6Address;
public class NetUtilsTest {
@ -418,4 +420,92 @@ public class NetUtilsTest {
assertTrue("It should pass! 31 bit prefix.", is31PrefixCidr);
}
}
@Test
public void testGetCidrNetMask() {
final String cidr = "10.10.0.0/16";
String netmask = NetUtils.getCidrNetmask("10.10.10.10/16");
assertTrue(cidr + " does not generate valid netmask " + netmask,NetUtils.isValidNetmask(netmask));
}
@Test
public void testGetCidrSubNet() {
final String cidr = "10.10.0.0/16";
String subnet = NetUtils.getCidrSubNet("10.10.10.10/16");
assertTrue(cidr + " does not contain " + subnet,NetUtils.isIpWithtInCidrRange(subnet, cidr));
}
@Test
public void testGetCidrSubNetWithWidth() {
final String cidr = "10.10.0.0/16";
String subnet = NetUtils.getCidrSubNet("10.10.10.10", 16);
assertTrue(cidr + " does not contain " + subnet,NetUtils.isIpWithtInCidrRange(subnet, cidr));
}
@Test
public void testIsValidCidrSize() {
final String cidrsize = "16";
long netbits = NetUtils.getCidrSizeFromString(cidrsize);
assertTrue(" does not compute " + cidrsize,netbits == 16);
}
@Test(expected=CloudRuntimeException.class)
public void testIsInvalidCidrSize() {
final String cidrsize = "33";
long netbits = NetUtils.getCidrSizeFromString(cidrsize);
assertTrue(" does not compute " + cidrsize,netbits == 16);
}
@Test(expected=CloudRuntimeException.class)
public void testIsInvalidCidrString() {
final String cidrsize = "ggg";
long netbits = NetUtils.getCidrSizeFromString(cidrsize);
assertTrue(" does not compute " + cidrsize,netbits == 16);
}
@Test
public void testCidrToLongArray() {
final String cidr = "10.192.10.10/10";
Long[] netbits = NetUtils.cidrToLong(cidr);
assertEquals("unexpected cidrsize " + netbits[1],10l, netbits[1].longValue());
assertEquals("(un)expected <" + 0x0ac00000L + "> netaddress " + netbits[0].longValue(),netbits[0].longValue(),0x0ac00000l);
}
@Test
public void testNetmaskFromCidr() {
long mask = NetUtils.netMaskFromCidr(1l);
assertEquals("mask not right: " + mask, 0x80000000, mask);
mask = NetUtils.netMaskFromCidr(32l);
assertEquals("mask not right: " + mask, 0xffffffff, mask);
}
@Test
public void testIsCidrsNotEmptyWithNullCidrs() {
assertEquals(false, NetUtils.areCidrsNotEmpty(null, null));
}
@Test
public void testIsCidrsNotEmptyWithEmptyCidrs() {
assertEquals(false, NetUtils.areCidrsNotEmpty("", " "));
}
@Test
public void testIsCidrsNotEmpty() {
assertEquals(true, NetUtils.areCidrsNotEmpty("10.10.0.0/16", "10.1.2.3/16"));
}
@Test
public void testIsNetowrkASubsetOrSupersetOfNetworkBWithEmptyValues() {
assertEquals(SupersetOrSubset.errorInCidrFormat, NetUtils.isNetowrkASubsetOrSupersetOfNetworkB("", null));
}
@Test
public void testIsNetworkAWithinNetworkBWithEmptyValues() {
assertEquals(false, NetUtils.isNetworkAWithinNetworkB("", null));
}
@Test
public void testIsNetworksOverlapWithEmptyValues() {
assertEquals(false, NetUtils.isNetworksOverlap("", null));
}
}