From 87a408a8466134c9c329871d921a8d8a8a76b126 Mon Sep 17 00:00:00 2001 From: Bradford Wetmore Date: Fri, 22 Aug 2008 18:48:00 -0700 Subject: [PATCH 01/23] 6497740: Limit the size of RSA public keys Reviewed-by: andreas, valeriep, vinnie --- .../security/pkcs11/P11KeyPairGenerator.java | 59 +++++--- .../sun/security/pkcs11/P11KeyStore.java | 11 ++ .../sun/security/pkcs11/P11RSAKeyFactory.java | 20 ++- .../sun/security/pkcs11/SunPKCS11.java | 89 ++++++++---- .../sun/security/rsa/RSAKeyFactory.java | 135 +++++++++++------- .../sun/security/rsa/RSAKeyPairGenerator.java | 59 ++++---- .../security/rsa/RSAPrivateCrtKeyImpl.java | 7 +- .../sun/security/rsa/RSAPrivateKeyImpl.java | 4 +- .../sun/security/rsa/RSAPublicKeyImpl.java | 9 +- .../security/mscapi/RSAKeyPairGenerator.java | 44 +++--- .../sun/security/mscapi/RSASignature.java | 30 ++-- 11 files changed, 299 insertions(+), 168 deletions(-) diff --git a/jdk/src/share/classes/sun/security/pkcs11/P11KeyPairGenerator.java b/jdk/src/share/classes/sun/security/pkcs11/P11KeyPairGenerator.java index 701e104d529..1027df9ddb7 100644 --- a/jdk/src/share/classes/sun/security/pkcs11/P11KeyPairGenerator.java +++ b/jdk/src/share/classes/sun/security/pkcs11/P11KeyPairGenerator.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2006 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2003-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -38,6 +38,8 @@ import static sun.security.pkcs11.TemplateManager.*; import sun.security.pkcs11.wrapper.*; import static sun.security.pkcs11.wrapper.PKCS11Constants.*; +import sun.security.rsa.RSAKeyFactory; + /** * KeyPairGenerator implementation class. This class currently supports * RSA, DSA, DH, and EC. @@ -66,7 +68,7 @@ final class P11KeyPairGenerator extends KeyPairGeneratorSpi { private AlgorithmParameterSpec params; // for RSA, selected or default value of public exponent, always valid - private BigInteger rsaPublicExponent; + private BigInteger rsaPublicExponent = RSAKeyGenParameterSpec.F4; // SecureRandom instance, if specified in init private SecureRandom random; @@ -88,19 +90,19 @@ final class P11KeyPairGenerator extends KeyPairGeneratorSpi { public void initialize(int keySize, SecureRandom random) { token.ensureValid(); try { - checkKeySize(keySize); + checkKeySize(keySize, null); } catch (InvalidAlgorithmParameterException e) { throw new InvalidParameterException(e.getMessage()); } this.keySize = keySize; this.params = null; this.random = random; - this.rsaPublicExponent = RSAKeyGenParameterSpec.F4; if (algorithm.equals("EC")) { params = P11ECKeyFactory.getECParameterSpec(keySize); if (params == null) { - throw new InvalidParameterException - ("No EC parameters available for key size " + keySize + " bits"); + throw new InvalidParameterException( + "No EC parameters available for key size " + + keySize + " bits"); } } } @@ -115,8 +117,10 @@ final class P11KeyPairGenerator extends KeyPairGeneratorSpi { ("DHParameterSpec required for Diffie-Hellman"); } DHParameterSpec dhParams = (DHParameterSpec)params; - this.keySize = dhParams.getP().bitLength(); - this.params = params; + int tmpKeySize = dhParams.getP().bitLength(); + checkKeySize(tmpKeySize, dhParams); + this.keySize = tmpKeySize; + this.params = dhParams; // XXX sanity check params } else if (algorithm.equals("RSA")) { if (params instanceof RSAKeyGenParameterSpec == false) { @@ -124,7 +128,9 @@ final class P11KeyPairGenerator extends KeyPairGeneratorSpi { ("RSAKeyGenParameterSpec required for RSA"); } RSAKeyGenParameterSpec rsaParams = (RSAKeyGenParameterSpec)params; - this.keySize = rsaParams.getKeysize(); + int tmpKeySize = rsaParams.getKeysize(); + checkKeySize(tmpKeySize, rsaParams); + this.keySize = tmpKeySize; this.params = null; this.rsaPublicExponent = rsaParams.getPublicExponent(); // XXX sanity check params @@ -134,13 +140,16 @@ final class P11KeyPairGenerator extends KeyPairGeneratorSpi { ("DSAParameterSpec required for DSA"); } DSAParameterSpec dsaParams = (DSAParameterSpec)params; - this.keySize = dsaParams.getP().bitLength(); - this.params = params; + int tmpKeySize = dsaParams.getP().bitLength(); + checkKeySize(tmpKeySize, dsaParams); + this.keySize = tmpKeySize; + this.params = dsaParams; // XXX sanity check params } else if (algorithm.equals("EC")) { ECParameterSpec ecParams; if (params instanceof ECParameterSpec) { - ecParams = P11ECKeyFactory.getECParameterSpec((ECParameterSpec)params); + ecParams = P11ECKeyFactory.getECParameterSpec( + (ECParameterSpec)params); if (ecParams == null) { throw new InvalidAlgorithmParameterException ("Unsupported curve: " + params); @@ -156,16 +165,17 @@ final class P11KeyPairGenerator extends KeyPairGeneratorSpi { throw new InvalidAlgorithmParameterException ("ECParameterSpec or ECGenParameterSpec required for EC"); } - this.keySize = ecParams.getCurve().getField().getFieldSize(); + int tmpKeySize = ecParams.getCurve().getField().getFieldSize(); + checkKeySize(tmpKeySize, ecParams); + this.keySize = tmpKeySize; this.params = ecParams; } else { throw new ProviderException("Unknown algorithm: " + algorithm); } this.random = random; - checkKeySize(keySize); } - private void checkKeySize(int keySize) + private void checkKeySize(int keySize, AlgorithmParameterSpec params) throws InvalidAlgorithmParameterException { if (algorithm.equals("EC")) { if (keySize < 112) { @@ -178,13 +188,28 @@ final class P11KeyPairGenerator extends KeyPairGeneratorSpi { ("Key size must be at most 2048 bit"); } return; + } else if (algorithm.equals("RSA")) { + BigInteger tmpExponent = rsaPublicExponent; + if (params != null) { + // Already tested for instanceof RSAKeyGenParameterSpec above + tmpExponent = + ((RSAKeyGenParameterSpec)params).getPublicExponent(); + } + try { + // This provider supports 64K or less. + RSAKeyFactory.checkKeyLengths(keySize, tmpExponent, + 512, 64 * 1024); + } catch (InvalidKeyException e) { + throw new InvalidAlgorithmParameterException(e.getMessage()); + } + return; } + if (keySize < 512) { throw new InvalidAlgorithmParameterException ("Key size must be at least 512 bit"); } - if (algorithm.equals("RSA") || - (algorithm.equals("DH") && (params != null))) { + if (algorithm.equals("DH") && (params != null)) { // sanity check, nobody really wants keys this large if (keySize > 64 * 1024) { throw new InvalidAlgorithmParameterException diff --git a/jdk/src/share/classes/sun/security/pkcs11/P11KeyStore.java b/jdk/src/share/classes/sun/security/pkcs11/P11KeyStore.java index 839a2f3ba64..b0e20711dc8 100644 --- a/jdk/src/share/classes/sun/security/pkcs11/P11KeyStore.java +++ b/jdk/src/share/classes/sun/security/pkcs11/P11KeyStore.java @@ -80,6 +80,8 @@ import static sun.security.pkcs11.P11Util.*; import sun.security.pkcs11.wrapper.*; import static sun.security.pkcs11.wrapper.PKCS11Constants.*; +import sun.security.rsa.RSAKeyFactory; + final class P11KeyStore extends KeyStoreSpi { private static final CK_ATTRIBUTE ATTR_CLASS_CERT = @@ -1328,6 +1330,15 @@ final class P11KeyStore extends KeyStoreSpi { BigInteger modulus = attrs[0].getBigInteger(); keyLength = modulus.bitLength(); + // This check will combine our "don't care" values here + // with the system-wide min/max values. + try { + RSAKeyFactory.checkKeyLengths(keyLength, null, + -1, Integer.MAX_VALUE); + } catch (InvalidKeyException e) { + throw new KeyStoreException(e.getMessage()); + } + return P11Key.privateKey(session, oHandle, keyType, diff --git a/jdk/src/share/classes/sun/security/pkcs11/P11RSAKeyFactory.java b/jdk/src/share/classes/sun/security/pkcs11/P11RSAKeyFactory.java index 5f3cbbf93c9..1bd764d326e 100644 --- a/jdk/src/share/classes/sun/security/pkcs11/P11RSAKeyFactory.java +++ b/jdk/src/share/classes/sun/security/pkcs11/P11RSAKeyFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2003 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2003-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -35,6 +35,8 @@ import static sun.security.pkcs11.TemplateManager.*; import sun.security.pkcs11.wrapper.*; import static sun.security.pkcs11.wrapper.PKCS11Constants.*; +import sun.security.rsa.RSAKeyFactory; + /** * RSA KeyFactory implemenation. * @@ -131,6 +133,9 @@ final class P11RSAKeyFactory extends P11KeyFactory { } catch (PKCS11Exception e) { throw new InvalidKeySpecException ("Could not create RSA public key", e); + } catch (InvalidKeyException e) { + throw new InvalidKeySpecException + ("Could not create RSA public key", e); } } @@ -175,11 +180,15 @@ final class P11RSAKeyFactory extends P11KeyFactory { } catch (PKCS11Exception e) { throw new InvalidKeySpecException ("Could not create RSA private key", e); + } catch (InvalidKeyException e) { + throw new InvalidKeySpecException + ("Could not create RSA private key", e); } } private PublicKey generatePublic(BigInteger n, BigInteger e) - throws PKCS11Exception { + throws PKCS11Exception, InvalidKeyException { + RSAKeyFactory.checkKeyLengths(n.bitLength(), e, -1, 64 * 1024); CK_ATTRIBUTE[] attributes = new CK_ATTRIBUTE[] { new CK_ATTRIBUTE(CKA_CLASS, CKO_PUBLIC_KEY), new CK_ATTRIBUTE(CKA_KEY_TYPE, CKK_RSA), @@ -200,7 +209,8 @@ final class P11RSAKeyFactory extends P11KeyFactory { } private PrivateKey generatePrivate(BigInteger n, BigInteger d) - throws PKCS11Exception { + throws PKCS11Exception, InvalidKeyException { + RSAKeyFactory.checkKeyLengths(n.bitLength(), null, -1, 64 * 1024); CK_ATTRIBUTE[] attributes = new CK_ATTRIBUTE[] { new CK_ATTRIBUTE(CKA_CLASS, CKO_PRIVATE_KEY), new CK_ATTRIBUTE(CKA_KEY_TYPE, CKK_RSA), @@ -222,7 +232,9 @@ final class P11RSAKeyFactory extends P11KeyFactory { private PrivateKey generatePrivate(BigInteger n, BigInteger e, BigInteger d, BigInteger p, BigInteger q, BigInteger pe, - BigInteger qe, BigInteger coeff) throws PKCS11Exception { + BigInteger qe, BigInteger coeff) throws PKCS11Exception, + InvalidKeyException { + RSAKeyFactory.checkKeyLengths(n.bitLength(), e, -1, 64 * 1024); CK_ATTRIBUTE[] attributes = new CK_ATTRIBUTE[] { new CK_ATTRIBUTE(CKA_CLASS, CKO_PRIVATE_KEY), new CK_ATTRIBUTE(CKA_KEY_TYPE, CKK_RSA), diff --git a/jdk/src/share/classes/sun/security/pkcs11/SunPKCS11.java b/jdk/src/share/classes/sun/security/pkcs11/SunPKCS11.java index b6eb1c258f4..6d0e045f2ee 100644 --- a/jdk/src/share/classes/sun/security/pkcs11/SunPKCS11.java +++ b/jdk/src/share/classes/sun/security/pkcs11/SunPKCS11.java @@ -120,11 +120,13 @@ public final class SunPKCS11 extends AuthProvider { } /** - * @deprecated use new SunPKCS11(String) or new SunPKCS11(InputStream) instead + * @deprecated use new SunPKCS11(String) or new SunPKCS11(InputStream) + * instead */ @Deprecated public SunPKCS11(String configName, InputStream configStream) { - super("SunPKCS11-" + Config.getConfig(configName, configStream).getName(), + super("SunPKCS11-" + + Config.getConfig(configName, configStream).getName(), 1.7d, Config.getConfig(configName, configStream).getDescription()); this.configName = configName; this.config = Config.removeConfig(configName); @@ -153,7 +155,8 @@ public final class SunPKCS11 extends AuthProvider { // // If we are in Secmod mode and configured to use either the // nssKeyStore or the nssTrustAnchors module, we automatically - // switch to using the NSS trust attributes for trusted certs (KeyStore). + // switch to using the NSS trust attributes for trusted certs + // (KeyStore). // if (useSecmod) { @@ -168,33 +171,40 @@ public final class SunPKCS11 extends AuthProvider { if (secmod.isInitialized()) { if (nssSecmodDirectory != null) { String s = secmod.getConfigDir(); - if ((s != null) && (s.equals(nssSecmodDirectory) == false)) { + if ((s != null) && + (s.equals(nssSecmodDirectory) == false)) { throw new ProviderException("Secmod directory " + nssSecmodDirectory - + " invalid, NSS already initialized with " + s); + + " invalid, NSS already initialized with " + + s); } } if (nssLibraryDirectory != null) { String s = secmod.getLibDir(); - if ((s != null) && (s.equals(nssLibraryDirectory) == false)) { + if ((s != null) && + (s.equals(nssLibraryDirectory) == false)) { throw new ProviderException("NSS library directory " + nssLibraryDirectory - + " invalid, NSS already initialized with " + s); + + " invalid, NSS already initialized with " + + s); } } } else { if (nssDbMode != DbMode.NO_DB) { if (nssSecmodDirectory == null) { - throw new ProviderException("Secmod not initialized and " - + "nssSecmodDirectory not specified"); + throw new ProviderException( + "Secmod not initialized and " + + "nssSecmodDirectory not specified"); } } else { if (nssSecmodDirectory != null) { - throw new ProviderException - ("nssSecmodDirectory must not be specified in noDb mode"); + throw new ProviderException( + "nssSecmodDirectory must not be " + + "specified in noDb mode"); } } - secmod.initialize(nssDbMode, nssSecmodDirectory, nssLibraryDirectory); + secmod.initialize(nssDbMode, nssSecmodDirectory, + nssLibraryDirectory); } } catch (IOException e) { // XXX which exception to throw @@ -211,7 +221,8 @@ public final class SunPKCS11 extends AuthProvider { if (nssModule != null) { moduleName = "fips"; } else { - moduleName = (nssDbMode == DbMode.NO_DB) ? "crypto" : "keystore"; + moduleName = (nssDbMode == DbMode.NO_DB) ? + "crypto" : "keystore"; } } if (moduleName.equals("fips")) { @@ -253,10 +264,12 @@ public final class SunPKCS11 extends AuthProvider { + ": only " + k + " external NSS modules available"); } } else { - throw new ProviderException("Unknown NSS module: " + moduleName); + throw new ProviderException( + "Unknown NSS module: " + moduleName); } if (nssModule == null) { - throw new ProviderException("NSS module not available: " + moduleName); + throw new ProviderException( + "NSS module not available: " + moduleName); } if (nssModule.hasInitializedProvider()) { throw new ProviderException("Secmod module already configured"); @@ -296,8 +309,9 @@ public final class SunPKCS11 extends AuthProvider { initArgs.flags = CKF_OS_LOCKING_OK; PKCS11 tmpPKCS11; try { - tmpPKCS11 = PKCS11.getInstance - (library, functionList, initArgs, config.getOmitInitialize()); + tmpPKCS11 = PKCS11.getInstance( + library, functionList, initArgs, + config.getOmitInitialize()); } catch (PKCS11Exception e) { if (debug != null) { debug.println("Multi-threaded initialization failed: " + e); @@ -312,8 +326,8 @@ public final class SunPKCS11 extends AuthProvider { } else { initArgs.flags = 0; } - tmpPKCS11 = PKCS11.getInstance - (library, functionList, initArgs, config.getOmitInitialize()); + tmpPKCS11 = PKCS11.getInstance(library, + functionList, initArgs, config.getOmitInitialize()); } p11 = tmpPKCS11; @@ -336,8 +350,10 @@ public final class SunPKCS11 extends AuthProvider { System.out.println("Slots with tokens: " + toString(slots)); } if (slotID < 0) { - if ((slotListIndex < 0) || (slotListIndex >= slots.length)) { - throw new ProviderException("slotListIndex is " + slotListIndex + if ((slotListIndex < 0) + || (slotListIndex >= slots.length)) { + throw new ProviderException("slotListIndex is " + + slotListIndex + " but token only has " + slots.length + " slots"); } slotID = slots[slotListIndex]; @@ -575,12 +591,15 @@ public final class SunPKCS11 extends AuthProvider { d(KF, "DH", P11DHKeyFactory, s("DiffieHellman"), m(CKM_DH_PKCS_KEY_PAIR_GEN, CKM_DH_PKCS_DERIVE)); d(KF, "EC", P11DHKeyFactory, - m(CKM_EC_KEY_PAIR_GEN, CKM_ECDH1_DERIVE, CKM_ECDSA, CKM_ECDSA_SHA1)); + m(CKM_EC_KEY_PAIR_GEN, CKM_ECDH1_DERIVE, + CKM_ECDSA, CKM_ECDSA_SHA1)); // AlgorithmParameters for EC. // Only needed until we have an EC implementation in the SUN provider. - d(AGP, "EC", "sun.security.ec.ECParameters", s("1.2.840.10045.2.1"), - m(CKM_EC_KEY_PAIR_GEN, CKM_ECDH1_DERIVE, CKM_ECDSA, CKM_ECDSA_SHA1)); + d(AGP, "EC", "sun.security.ec.ECParameters", + s("1.2.840.10045.2.1"), + m(CKM_EC_KEY_PAIR_GEN, CKM_ECDH1_DERIVE, + CKM_ECDSA, CKM_ECDSA_SHA1)); d(KA, "DH", P11KeyAgreement, s("DiffieHellman"), m(CKM_DH_PKCS_DERIVE)); @@ -654,12 +673,16 @@ public final class SunPKCS11 extends AuthProvider { d(SIG, "SHA512withRSA", P11Signature, m(CKM_SHA512_RSA_PKCS, CKM_RSA_PKCS, CKM_RSA_X_509)); - d(KG, "SunTlsRsaPremasterSecret", "sun.security.pkcs11.P11TlsRsaPremasterSecretGenerator", + d(KG, "SunTlsRsaPremasterSecret", + "sun.security.pkcs11.P11TlsRsaPremasterSecretGenerator", m(CKM_SSL3_PRE_MASTER_KEY_GEN, CKM_TLS_PRE_MASTER_KEY_GEN)); - d(KG, "SunTlsMasterSecret", "sun.security.pkcs11.P11TlsMasterSecretGenerator", + d(KG, "SunTlsMasterSecret", + "sun.security.pkcs11.P11TlsMasterSecretGenerator", m(CKM_SSL3_MASTER_KEY_DERIVE, CKM_TLS_MASTER_KEY_DERIVE, - CKM_SSL3_MASTER_KEY_DERIVE_DH, CKM_TLS_MASTER_KEY_DERIVE_DH)); - d(KG, "SunTlsKeyMaterial", "sun.security.pkcs11.P11TlsKeyMaterialGenerator", + CKM_SSL3_MASTER_KEY_DERIVE_DH, + CKM_TLS_MASTER_KEY_DERIVE_DH)); + d(KG, "SunTlsKeyMaterial", + "sun.security.pkcs11.P11TlsKeyMaterialGenerator", m(CKM_SSL3_KEY_AND_MAC_DERIVE, CKM_TLS_KEY_AND_MAC_DERIVE)); d(KG, "SunTlsPrf", "sun.security.pkcs11.P11TlsPrfGenerator", m(CKM_TLS_PRF, CKM_NSS_TLS_PRF_GENERAL)); @@ -773,6 +796,13 @@ public final class SunPKCS11 extends AuthProvider { System.out.println(token.tokenInfo); } long[] supportedMechanisms = p11.C_GetMechanismList(slotID); + + // Create a map from the various Descriptors to the "most + // preferred" mechanism that was defined during the + // static initialization. For example, DES/CBC/PKCS5Padding + // could be mapped to CKM_DES_CBC_PAD or CKM_DES_CBC. Prefer + // the earliest entry. When asked for "DES/CBC/PKCS5Padding", we + // return a CKM_DES_CBC_PAD. final Map supportedAlgs = new HashMap(); for (int i = 0; i < supportedMechanisms.length; i++) { @@ -807,6 +837,9 @@ public final class SunPKCS11 extends AuthProvider { supportedAlgs.put(d, integerMech); continue; } + // See if there is something "more preferred" + // than what we currently have in the supportedAlgs + // map. int intOldMech = oldMech.intValue(); for (int j = 0; j < d.mechanisms.length; j++) { int nextMech = d.mechanisms[j]; diff --git a/jdk/src/share/classes/sun/security/rsa/RSAKeyFactory.java b/jdk/src/share/classes/sun/security/rsa/RSAKeyFactory.java index 17ce8cde639..d877f28fb54 100644 --- a/jdk/src/share/classes/sun/security/rsa/RSAKeyFactory.java +++ b/jdk/src/share/classes/sun/security/rsa/RSAKeyFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2006 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2003-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -31,6 +31,8 @@ import java.security.*; import java.security.interfaces.*; import java.security.spec.*; +import sun.security.action.GetPropertyAction; + /** * KeyFactory for RSA keys. Keys must be instances of PublicKey or PrivateKey * and getAlgorithm() must return "RSA". For such keys, it supports conversion @@ -68,6 +70,24 @@ public final class RSAKeyFactory extends KeyFactorySpi { private final static Class x509KeySpecClass = X509EncodedKeySpec.class; private final static Class pkcs8KeySpecClass = PKCS8EncodedKeySpec.class; + public final static int MIN_MODLEN = 512; + public final static int MAX_MODLEN = 16384; + + /* + * If the modulus length is above this value, restrict the size of + * the exponent to something that can be reasonably computed. We + * could simply hardcode the exp len to something like 64 bits, but + * this approach allows flexibility in case impls would like to use + * larger module and exponent values. + */ + public final static int MAX_MODLEN_RESTRICT_EXP = 3072; + public final static int MAX_RESTRICTED_EXPLEN = 64; + + private static final boolean restrictExpLen = + "true".equalsIgnoreCase(AccessController.doPrivileged( + new GetPropertyAction( + "sun.security.rsa.restrictRSAExponent", "true"))); + // instance used for static translateKey(); private final static RSAKeyFactory INSTANCE = new RSAKeyFactory(); @@ -76,74 +96,79 @@ public final class RSAKeyFactory extends KeyFactorySpi { } /** - * Static method to convert Key into a useable instance of - * RSAPublicKey or RSAPrivate(Crt)Key. Check the key and convert it - * to a SunRsaSign key if necessary. If the key is not an RSA key - * or cannot be used, throw an InvalidKeyException. - * - * The difference between this method and engineTranslateKey() is that - * we do not convert keys of other providers that are already an - * instance of RSAPublicKey or RSAPrivate(Crt)Key. + * Static method to convert Key into an instance of RSAPublicKeyImpl + * or RSAPrivate(Crt)KeyImpl. If the key is not an RSA key or cannot be + * used, throw an InvalidKeyException. * * Used by RSASignature and RSACipher. */ public static RSAKey toRSAKey(Key key) throws InvalidKeyException { - if (key instanceof RSAKey) { - RSAKey rsaKey = (RSAKey)key; - checkKey(rsaKey); - return rsaKey; + if ((key instanceof RSAPrivateKeyImpl) || + (key instanceof RSAPrivateCrtKeyImpl) || + (key instanceof RSAPublicKeyImpl)) { + return (RSAKey)key; } else { return (RSAKey)INSTANCE.engineTranslateKey(key); } } - /** - * Check that the given RSA key is valid. + /* + * Single test entry point for all of the mechanisms in the SunRsaSign + * provider (RSA*KeyImpls). All of the tests are the same. + * + * For compatibility, we round up to the nearest byte here: + * some Key impls might pass in a value within a byte of the + * real value. */ - private static void checkKey(RSAKey key) throws InvalidKeyException { - // check for subinterfaces, omit additional checks for our keys - if (key instanceof RSAPublicKey) { - if (key instanceof RSAPublicKeyImpl) { - return; - } - } else if (key instanceof RSAPrivateKey) { - if ((key instanceof RSAPrivateCrtKeyImpl) - || (key instanceof RSAPrivateKeyImpl)) { - return; - } - } else { - throw new InvalidKeyException("Neither a public nor a private key"); - } - // RSAKey does not extend Key, so we need to do a cast - String keyAlg = ((Key)key).getAlgorithm(); - if (keyAlg.equals("RSA") == false) { - throw new InvalidKeyException("Not an RSA key: " + keyAlg); - } - BigInteger modulus; - // some providers implement RSAKey for keys where the values are - // not accessible (although they should). Detect those here - // for a more graceful failure. - try { - modulus = key.getModulus(); - if (modulus == null) { - throw new InvalidKeyException("Modulus is missing"); - } - } catch (RuntimeException e) { - throw new InvalidKeyException(e); - } - checkKeyLength(modulus); + static void checkRSAProviderKeyLengths(int modulusLen, BigInteger exponent) + throws InvalidKeyException { + checkKeyLengths(((modulusLen + 7) & ~7), exponent, + RSAKeyFactory.MIN_MODLEN, Integer.MAX_VALUE); } /** - * Check the length of the modulus of an RSA key. We only support keys - * at least 505 bits long. + * Check the length of an RSA key modulus/exponent to make sure it + * is not too short or long. Some impls have their own min and + * max key sizes that may or may not match with a system defined value. + * + * @param modulusLen the bit length of the RSA modulus. + * @param exponent the RSA exponent + * @param minModulusLen if > 0, check to see if modulusLen is at + * least this long, otherwise unused. + * @param maxModulusLen caller will allow this max number of bits. + * Allow the smaller of the system-defined maximum and this param. + * + * @throws InvalidKeyException if any of the values are unacceptable. */ - static void checkKeyLength(BigInteger modulus) throws InvalidKeyException { - if (modulus.bitLength() < 505) { - // some providers may generate slightly shorter keys - // accept them if the encoding is at least 64 bytes long - throw new InvalidKeyException - ("RSA keys must be at least 512 bits long"); + public static void checkKeyLengths(int modulusLen, BigInteger exponent, + int minModulusLen, int maxModulusLen) throws InvalidKeyException { + + if ((minModulusLen > 0) && (modulusLen < (minModulusLen))) { + throw new InvalidKeyException( "RSA keys must be at least " + + minModulusLen + " bits long"); + } + + // Even though our policy file may allow this, we don't want + // either value (mod/exp) to be too big. + + int maxLen = Math.min(maxModulusLen, MAX_MODLEN); + + // If a RSAPrivateKey/RSAPublicKey, make sure the + // modulus len isn't too big. + if (modulusLen > maxLen) { + throw new InvalidKeyException( + "RSA keys must be no longer than " + maxLen + " bits"); + } + + // If a RSAPublicKey, make sure the exponent isn't too big. + if (restrictExpLen && (exponent != null) && + (modulusLen > MAX_MODLEN_RESTRICT_EXP) && + (exponent.bitLength() > MAX_RESTRICTED_EXPLEN)) { + throw new InvalidKeyException( + "RSA exponents can be no longer than " + + MAX_RESTRICTED_EXPLEN + " bits " + + " if modulus is greater than " + + MAX_MODLEN_RESTRICT_EXP + " bits"); } } diff --git a/jdk/src/share/classes/sun/security/rsa/RSAKeyPairGenerator.java b/jdk/src/share/classes/sun/security/rsa/RSAKeyPairGenerator.java index f800dec543b..2898a3d1f51 100644 --- a/jdk/src/share/classes/sun/security/rsa/RSAKeyPairGenerator.java +++ b/jdk/src/share/classes/sun/security/rsa/RSAKeyPairGenerator.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2004 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2003-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -47,7 +47,7 @@ public final class RSAKeyPairGenerator extends KeyPairGeneratorSpi { // public exponent to use private BigInteger publicExponent; - // size of the key to generate, >= 512 + // size of the key to generate, >= RSAKeyFactory.MIN_MODLEN private int keySize; // PRNG to use @@ -60,15 +60,16 @@ public final class RSAKeyPairGenerator extends KeyPairGeneratorSpi { // initialize the generator. See JCA doc public void initialize(int keySize, SecureRandom random) { - if (keySize < 512) { - throw new InvalidParameterException - ("Key size must be at least 512 bits"); - } - if (keySize > 64 * 1024) { - // do not allow unreasonably large key sizes, probably user error - throw new InvalidParameterException - ("Key size must be 65536 bits or less"); + + // do not allow unreasonably small or large key sizes, + // probably user error + try { + RSAKeyFactory.checkKeyLengths(keySize, RSAKeyGenParameterSpec.F4, + 512, 64 * 1024); + } catch (InvalidKeyException e) { + throw new InvalidParameterException(e.getMessage()); } + this.keySize = keySize; this.random = random; this.publicExponent = RSAKeyGenParameterSpec.F4; @@ -77,35 +78,41 @@ public final class RSAKeyPairGenerator extends KeyPairGeneratorSpi { // second initialize method. See JCA doc. public void initialize(AlgorithmParameterSpec params, SecureRandom random) throws InvalidAlgorithmParameterException { + if (params instanceof RSAKeyGenParameterSpec == false) { throw new InvalidAlgorithmParameterException ("Params must be instance of RSAKeyGenParameterSpec"); } + RSAKeyGenParameterSpec rsaSpec = (RSAKeyGenParameterSpec)params; - keySize = rsaSpec.getKeysize(); - publicExponent = rsaSpec.getPublicExponent(); - this.random = random; - if (keySize < 512) { - throw new InvalidAlgorithmParameterException - ("Key size must be at least 512 bits"); - } - if (keySize > 64 * 1024) { - // do not allow unreasonably large key sizes, probably user error - throw new InvalidAlgorithmParameterException - ("Key size must be 65536 bits or less"); - } - if (publicExponent == null) { - publicExponent = RSAKeyGenParameterSpec.F4; + int tmpKeySize = rsaSpec.getKeysize(); + BigInteger tmpPublicExponent = rsaSpec.getPublicExponent(); + + if (tmpPublicExponent == null) { + tmpPublicExponent = RSAKeyGenParameterSpec.F4; } else { - if (publicExponent.compareTo(RSAKeyGenParameterSpec.F0) < 0) { + if (tmpPublicExponent.compareTo(RSAKeyGenParameterSpec.F0) < 0) { throw new InvalidAlgorithmParameterException ("Public exponent must be 3 or larger"); } - if (publicExponent.bitLength() > keySize) { + if (tmpPublicExponent.bitLength() > tmpKeySize) { throw new InvalidAlgorithmParameterException ("Public exponent must be smaller than key size"); } } + + // do not allow unreasonably large key sizes, probably user error + try { + RSAKeyFactory.checkKeyLengths(tmpKeySize, tmpPublicExponent, + 512, 64 * 1024); + } catch (InvalidKeyException e) { + throw new InvalidAlgorithmParameterException( + "Invalid key sizes", e); + } + + this.keySize = tmpKeySize; + this.publicExponent = tmpPublicExponent; + this.random = random; } // generate the keypair. See JCA doc diff --git a/jdk/src/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java b/jdk/src/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java index ac296b49f60..640972a2495 100644 --- a/jdk/src/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java +++ b/jdk/src/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2005 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2003-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -89,7 +89,7 @@ public final class RSAPrivateCrtKeyImpl */ RSAPrivateCrtKeyImpl(byte[] encoded) throws InvalidKeyException { decode(encoded); - RSAKeyFactory.checkKeyLength(n); + RSAKeyFactory.checkRSAProviderKeyLengths(n.bitLength(), e); } /** @@ -107,7 +107,8 @@ public final class RSAPrivateCrtKeyImpl this.pe = pe; this.qe = qe; this.coeff = coeff; - RSAKeyFactory.checkKeyLength(n); + RSAKeyFactory.checkRSAProviderKeyLengths(n.bitLength(), e); + // generate the encoding algid = rsaId; try { diff --git a/jdk/src/share/classes/sun/security/rsa/RSAPrivateKeyImpl.java b/jdk/src/share/classes/sun/security/rsa/RSAPrivateKeyImpl.java index 1f883f4205d..860e7706bc9 100644 --- a/jdk/src/share/classes/sun/security/rsa/RSAPrivateKeyImpl.java +++ b/jdk/src/share/classes/sun/security/rsa/RSAPrivateKeyImpl.java @@ -1,5 +1,5 @@ /* - * Copyright 2003 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2003-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -61,7 +61,7 @@ public final class RSAPrivateKeyImpl extends PKCS8Key implements RSAPrivateKey { RSAPrivateKeyImpl(BigInteger n, BigInteger d) throws InvalidKeyException { this.n = n; this.d = d; - RSAKeyFactory.checkKeyLength(n); + RSAKeyFactory.checkRSAProviderKeyLengths(n.bitLength(), null); // generate the encoding algid = RSAPrivateCrtKeyImpl.rsaId; try { diff --git a/jdk/src/share/classes/sun/security/rsa/RSAPublicKeyImpl.java b/jdk/src/share/classes/sun/security/rsa/RSAPublicKeyImpl.java index c500ca2dee4..6db08f6a393 100644 --- a/jdk/src/share/classes/sun/security/rsa/RSAPublicKeyImpl.java +++ b/jdk/src/share/classes/sun/security/rsa/RSAPublicKeyImpl.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2005 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2003-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -56,10 +56,11 @@ public final class RSAPublicKeyImpl extends X509Key implements RSAPublicKey { * Construct a key from its components. Used by the * RSAKeyFactory and the RSAKeyPairGenerator. */ - public RSAPublicKeyImpl(BigInteger n, BigInteger e) throws InvalidKeyException { + public RSAPublicKeyImpl(BigInteger n, BigInteger e) + throws InvalidKeyException { this.n = n; this.e = e; - RSAKeyFactory.checkKeyLength(n); + RSAKeyFactory.checkRSAProviderKeyLengths(n.bitLength(), e); // generate the encoding algid = RSAPrivateCrtKeyImpl.rsaId; try { @@ -80,7 +81,7 @@ public final class RSAPublicKeyImpl extends X509Key implements RSAPublicKey { */ public RSAPublicKeyImpl(byte[] encoded) throws InvalidKeyException { decode(encoded); - RSAKeyFactory.checkKeyLength(n); + RSAKeyFactory.checkRSAProviderKeyLengths(n.bitLength(), e); } // see JCA doc diff --git a/jdk/src/windows/classes/sun/security/mscapi/RSAKeyPairGenerator.java b/jdk/src/windows/classes/sun/security/mscapi/RSAKeyPairGenerator.java index 146d7c1632d..44dd25ab8cd 100644 --- a/jdk/src/windows/classes/sun/security/mscapi/RSAKeyPairGenerator.java +++ b/jdk/src/windows/classes/sun/security/mscapi/RSAKeyPairGenerator.java @@ -1,5 +1,5 @@ /* - * Copyright 2005 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2005-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -31,6 +31,7 @@ import java.security.spec.AlgorithmParameterSpec; import java.security.spec.RSAKeyGenParameterSpec; import sun.security.jca.JCAUtil; +import sun.security.rsa.RSAKeyFactory; /** * RSA keypair generator. @@ -43,8 +44,8 @@ import sun.security.jca.JCAUtil; public final class RSAKeyPairGenerator extends KeyPairGeneratorSpi { // Supported by Microsoft Base, Strong and Enhanced Cryptographic Providers - private static final int KEY_SIZE_MIN = 512; // disallow MSCAPI min. of 384 - private static final int KEY_SIZE_MAX = 16384; + static final int KEY_SIZE_MIN = 512; // disallow MSCAPI min. of 384 + static final int KEY_SIZE_MAX = 16384; private static final int KEY_SIZE_DEFAULT = 1024; // size of the key to generate, KEY_SIZE_MIN <= keySize <= KEY_SIZE_MAX @@ -59,7 +60,14 @@ public final class RSAKeyPairGenerator extends KeyPairGeneratorSpi { // random is always ignored public void initialize(int keySize, SecureRandom random) { - checkKeySize(keySize); + try { + RSAKeyFactory.checkKeyLengths(keySize, null, + KEY_SIZE_MIN, KEY_SIZE_MAX); + } catch (InvalidKeyException e) { + throw new InvalidParameterException(e.getMessage()); + } + + this.keySize = keySize; } // second initialize method. See JCA doc @@ -67,21 +75,31 @@ public final class RSAKeyPairGenerator extends KeyPairGeneratorSpi { public void initialize(AlgorithmParameterSpec params, SecureRandom random) throws InvalidAlgorithmParameterException { + int tmpSize; if (params == null) { - checkKeySize(KEY_SIZE_DEFAULT); - + tmpSize = KEY_SIZE_DEFAULT; } else if (params instanceof RSAKeyGenParameterSpec) { if (((RSAKeyGenParameterSpec) params).getPublicExponent() != null) { throw new InvalidAlgorithmParameterException ("Exponent parameter is not supported"); } - checkKeySize(((RSAKeyGenParameterSpec) params).getKeysize()); + tmpSize = ((RSAKeyGenParameterSpec) params).getKeysize(); } else { throw new InvalidAlgorithmParameterException ("Params must be an instance of RSAKeyGenParameterSpec"); } + + try { + RSAKeyFactory.checkKeyLengths(tmpSize, null, + KEY_SIZE_MIN, KEY_SIZE_MAX); + } catch (InvalidKeyException e) { + throw new InvalidAlgorithmParameterException( + "Invalid Key sizes", e); + } + + this.keySize = tmpSize; } // generate the keypair. See JCA doc @@ -95,18 +113,6 @@ public final class RSAKeyPairGenerator extends KeyPairGeneratorSpi { return new KeyPair(keys.getPublic(), keys.getPrivate()); } - private void checkKeySize(int keySize) throws InvalidParameterException { - if (keySize < KEY_SIZE_MIN) { - throw new InvalidParameterException - ("Key size must be at least " + KEY_SIZE_MIN + " bits"); - } - if (keySize > KEY_SIZE_MAX) { - throw new InvalidParameterException - ("Key size must be " + KEY_SIZE_MAX + " bits or less"); - } - this.keySize = keySize; - } - private static native RSAKeyPair generateRSAKeyPair(int keySize, String keyContainerName); } diff --git a/jdk/src/windows/classes/sun/security/mscapi/RSASignature.java b/jdk/src/windows/classes/sun/security/mscapi/RSASignature.java index 606423d53a4..982e1836a7d 100644 --- a/jdk/src/windows/classes/sun/security/mscapi/RSASignature.java +++ b/jdk/src/windows/classes/sun/security/mscapi/RSASignature.java @@ -1,5 +1,5 @@ /* - * Copyright 2005 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2005-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -38,6 +38,9 @@ import java.security.SecureRandom; import java.security.Signature; import java.security.SignatureSpi; import java.security.SignatureException; +import java.math.BigInteger; + +import sun.security.rsa.RSAKeyFactory; /** * RSA signature implementation. Supports RSA signing using PKCS#1 v1.5 padding. @@ -124,7 +127,16 @@ abstract class RSASignature extends java.security.SignatureSpi // convert key to MSCAPI format - byte[] modulusBytes = rsaKey.getModulus().toByteArray(); + BigInteger modulus = rsaKey.getModulus(); + BigInteger exponent = rsaKey.getPublicExponent(); + + // Check against the local and global values to make sure + // the sizes are ok. Round up to the nearest byte. + RSAKeyFactory.checkKeyLengths(((modulus.bitLength() + 7) & ~7), + exponent, -1, RSAKeyPairGenerator.KEY_SIZE_MAX); + + byte[] modulusBytes = modulus.toByteArray(); + byte[] exponentBytes = exponent.toByteArray(); // Adjust key length due to sign bit int keyBitLength = (modulusBytes[0] == 0) @@ -132,8 +144,7 @@ abstract class RSASignature extends java.security.SignatureSpi : modulusBytes.length * 8; byte[] keyBlob = generatePublicKeyBlob( - keyBitLength, modulusBytes, - rsaKey.getPublicExponent().toByteArray()); + keyBitLength, modulusBytes, exponentBytes); publicKey = importPublicKey(keyBlob, keyBitLength); @@ -166,12 +177,11 @@ abstract class RSASignature extends java.security.SignatureSpi } privateKey = (sun.security.mscapi.RSAPrivateKey) key; - // Determine byte length from bit length - int keySize = (privateKey.bitLength() + 7) >> 3; - - if (keySize < 64) - throw new InvalidKeyException( - "RSA keys must be at least 512 bits long"); + // Check against the local and global values to make sure + // the sizes are ok. Round up to nearest byte. + RSAKeyFactory.checkKeyLengths(((privateKey.bitLength() + 7) & ~7), + null, RSAKeyPairGenerator.KEY_SIZE_MIN, + RSAKeyPairGenerator.KEY_SIZE_MAX); if (needsReset) { messageDigest.reset(); From 6aab63dd371dfeb79ce7c3a9c70fd1758c7c4a9f Mon Sep 17 00:00:00 2001 From: Weijun Wang Date: Wed, 1 Oct 2008 10:01:45 +0800 Subject: [PATCH 02/23] 6588160: jaas krb5 client leaks OS-level UDP sockets (all platforms) Reviewed-by: jccollet, chegar --- .../classes/sun/security/krb5/KrbKdcReq.java | 38 ++++++++++--------- .../sun/security/krb5/internal/UDPClient.java | 3 ++ 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/jdk/src/share/classes/sun/security/krb5/KrbKdcReq.java b/jdk/src/share/classes/sun/security/krb5/KrbKdcReq.java index e9ff9cc04d5..259aedd66fa 100644 --- a/jdk/src/share/classes/sun/security/krb5/KrbKdcReq.java +++ b/jdk/src/share/classes/sun/security/krb5/KrbKdcReq.java @@ -274,27 +274,31 @@ public abstract class KrbKdcReq { + ",Attempt =" + i + ", #bytes=" + obuf.length); } - /* - * Send the data to the kdc. - */ + try { + /* + * Send the data to the kdc. + */ kdcClient.send(obuf); - /* - * And get a response. - */ - try { - ibuf = kdcClient.receive(); - break; - } catch (SocketTimeoutException se) { - if (DEBUG) { - System.out.println ("SocketTimeOutException with " + - "attempt: " + i); - } - if (i == DEFAULT_KDC_RETRY_LIMIT) { - ibuf = null; - throw se; + /* + * And get a response. + */ + try { + ibuf = kdcClient.receive(); + break; + } catch (SocketTimeoutException se) { + if (DEBUG) { + System.out.println ("SocketTimeOutException with " + + "attempt: " + i); + } + if (i == DEFAULT_KDC_RETRY_LIMIT) { + ibuf = null; + throw se; + } } + } finally { + kdcClient.close(); } } } diff --git a/jdk/src/share/classes/sun/security/krb5/internal/UDPClient.java b/jdk/src/share/classes/sun/security/krb5/internal/UDPClient.java index c9e440f1ef7..670df9210df 100644 --- a/jdk/src/share/classes/sun/security/krb5/internal/UDPClient.java +++ b/jdk/src/share/classes/sun/security/krb5/internal/UDPClient.java @@ -93,4 +93,7 @@ public class UDPClient { return data; } + public void close() { + dgSocket.close(); + } } From 3a7a9cc5577bd4ad27ae072ccd9b3bf65c637c02 Mon Sep 17 00:00:00 2001 From: Kumar Srinivasan Date: Thu, 4 Sep 2008 09:43:32 -0700 Subject: [PATCH 03/23] 6733959: Insufficient checks for "Main-Class" manifest entry in JAR files Fixes a buffer overrun problem with a very long Main-Class attribute. Reviewed-by: darcy --- jdk/src/share/bin/emessages.h | 1 + jdk/src/share/bin/java.c | 10 ++- jdk/test/tools/launcher/MultipleJRE.sh | 82 ++++++++++++++++++----- jdk/test/tools/launcher/ZipMeUp.java | 91 ++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 20 deletions(-) create mode 100644 jdk/test/tools/launcher/ZipMeUp.java diff --git a/jdk/src/share/bin/emessages.h b/jdk/src/share/bin/emessages.h index 03824bba5d1..6bfb8e16614 100644 --- a/jdk/src/share/bin/emessages.h +++ b/jdk/src/share/bin/emessages.h @@ -54,6 +54,7 @@ #define CLS_ERROR2 "Error: Failed to load Main Class: %s\n%s" #define CLS_ERROR3 "Error: No main method found in specified class.\n" GEN_ERROR #define CLS_ERROR4 "Error: Main method not public\n" GEN_ERROR +#define CLS_ERROR5 "Error: main-class: attribute exceeds system limits of %d bytes\n" GEN_ERROR #define CFG_WARN1 "Warning: %s VM not supported; %s VM will be used" #define CFG_WARN2 "Warning: No leading - on line %d of `%s'" diff --git a/jdk/src/share/bin/java.c b/jdk/src/share/bin/java.c index f7cbcdc95bc..b351fe3aa83 100644 --- a/jdk/src/share/bin/java.c +++ b/jdk/src/share/bin/java.c @@ -987,8 +987,14 @@ SelectVersion(int argc, char **argv, char **main_class) * to avoid locating, expanding and parsing the manifest extra * times. */ - if (info.main_class != NULL) - (void)JLI_StrCat(env_entry, info.main_class); + if (info.main_class != NULL) { + if (JLI_StrLen(info.main_class) <= MAXNAMELEN) { + (void)JLI_StrCat(env_entry, info.main_class); + } else { + ReportErrorMessage(CLS_ERROR5, MAXNAMELEN); + exit(1); + } + } (void)putenv(env_entry); ExecJRE(jre, new_argv); JLI_FreeManifest(); diff --git a/jdk/test/tools/launcher/MultipleJRE.sh b/jdk/test/tools/launcher/MultipleJRE.sh index b3e1cf764ed..a6dfaaa5253 100644 --- a/jdk/test/tools/launcher/MultipleJRE.sh +++ b/jdk/test/tools/launcher/MultipleJRE.sh @@ -1,14 +1,14 @@ # @test MultipleJRE.sh -# @bug 4811102 4953711 4955505 4956301 4991229 4998210 5018605 6387069 +# @bug 4811102 4953711 4955505 4956301 4991229 4998210 5018605 6387069 6733959 # @build PrintVersion # @build UglyPrintVersion +# @build ZipMeUp # @run shell MultipleJRE.sh # @summary Verify Multiple JRE version support # @author Joseph E. Kowalski - # -# Copyright 2003-2007 Sun Microsystems, Inc. All Rights Reserved. +# Copyright 2003-2008 Sun Microsystems, Inc. All Rights Reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -49,10 +49,25 @@ then exit 1 fi +JAVAEXE="$TESTJAVA/bin/java" JAVA="$TESTJAVA/bin/java -classpath $TESTCLASSES" JAR="$TESTJAVA/bin/jar" OS=`uname -s`; +# +# Tests whether we are on windows (true) or not. +# +IsWindows() { + case "$OS" in + Windows* | CYGWIN* ) + printf "true" + ;; + * ) + printf "false" + ;; + esac +} + # # Shell routine to test for the proper rejection of syntactically incorrect # version specifications. @@ -139,7 +154,6 @@ CreateUglyJar() { $PACKAGE/UglyPrintVersion.class } - # # Constructs a jar file with a fair number of "zip directory" entries and # the MANIFEST.MF entry at or near the end of that directory to validate @@ -262,6 +276,29 @@ LaunchVM() { fi } +# Tests very long Main-Class attribute in the jar +TestLongMainClass() { + JVER=$1 + if [ "$JVER" = "mklink" ]; then + JVER=XX + JDKXX=jdk/j2re$JVER + rm -rf jdk + mkdir jdk + ln -s $TESTJAVA $JDKXX + JAVA_VERSION_PATH="`pwd`/jdk" + export JAVA_VERSION_PATH + fi + $JAVAEXE -cp $TESTCLASSES ZipMeUp UglyBetty.jar 4097 + message="`$JAVAEXE -version:$JVER -jar UglyBetty.jar 2>&1`" + echo $message | grep "Error: main-class: attribute exceeds system limits" > /dev/null 2>&1 + if [ $? -ne 0 ]; then + printf "Long manifest test did not get expected error" + exit 1 + fi + unset JAVA_VERSION_PATH + rm -rf jdk +} + # # Main test sequence starts here # @@ -280,14 +317,11 @@ CreateJar "" "" LaunchVM "" "${RELEASE}" CreateJar "" "0" LaunchVM "" "${RELEASE}" -case "$OS" in - Windows* | CYGWIN* ) - MAXIMUM_PATH=255; - ;; - *) - MAXIMUM_PATH=1024; - ;; -esac +if [ `IsWindows` = "true" ]; then + MAXIMUM_PATH=255; +else + MAXIMUM_PATH=1024; +fi PATH_LENGTH=`printf "%s" "$UGLYCLASS" | wc -c` if [ ${PATH_LENGTH} -lt ${MAXIMUM_PATH} ]; then @@ -346,7 +380,6 @@ if [ -x /usr/bin/zipnote ]; then LaunchVM "" "${RELEASE}" fi - # # Throw some syntactically challenged (illegal) version specifiers at # the interface. Failure (of the launcher) is success for the test. @@ -357,15 +390,28 @@ TestSyntax "1.2.3-" # Ends with a separator TestSyntax "1.2+.3" # Embedded modifier TestSyntax "1.2.4+&1.2*&1++" # Long and invalid +# On windows we see if there is another jre installed, usually +# there is, then we test using that, otherwise links are created +# to get through to SelectVersion. +if [ `IsWindows` = "false" ]; then + TestLongMainClass "mklink" +else + $JAVAEXE -version:1.0+ + if [ $? -eq 0 ]; then + TestLongMainClass "1.0+" + else + printf "Warning: TestLongMainClass skipped as there is no" + printf "viable MJRE installed.\n" + fi +fi + # # Because scribbling in the registry can be rather destructive, only a # subset of the tests are run on Windows. # -case "$OS" in - Windows* | CYGWIN* ) - exit 0; - ;; -esac +if [ `IsWindows` = "true" ]; then + exit 0; +fi # # Additional version specifiers containing spaces. (Sigh, unable to diff --git a/jdk/test/tools/launcher/ZipMeUp.java b/jdk/test/tools/launcher/ZipMeUp.java new file mode 100644 index 00000000000..84d3f186e7e --- /dev/null +++ b/jdk/test/tools/launcher/ZipMeUp.java @@ -0,0 +1,91 @@ +/* + * Copyright 2008 Sun Microsystems, Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, + * CA 95054 USA or visit www.sun.com if you need additional information or + * have any questions. + */ + +/** + * A simple class to create our erring Jar with a very long Main-Class + * attribute in the manifest. + */ +import java.io.ByteArrayOutputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.PrintStream; +import java.util.zip.CRC32; +import java.util.zip.CheckedOutputStream; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; +public class ZipMeUp { + + static final CRC32 crc = new CRC32(); + + private static String SOME_KLASS = ".Some"; + + static byte[] getManifestAsBytes(int nchars) throws IOException { + crc.reset(); + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + CheckedOutputStream cos = new CheckedOutputStream(baos, crc); + PrintStream ps = new PrintStream(cos); + ps.println("Manifest-Version: 1.0"); + ps.print("Main-Class: "); + for (int i = 0 ; i < nchars - SOME_KLASS.length() ; i++) { + ps.print(i%10); + } + ps.println(SOME_KLASS); + cos.flush(); + cos.close(); + ps.close(); + return baos.toByteArray(); + } + + /** + * The arguments are: filename_to_create length + * @param args + * @throws java.lang.Exception + */ + public static void main(String...args) throws Exception { + FileOutputStream fos = new FileOutputStream(args[0]); + ZipOutputStream zos = new ZipOutputStream(fos); + byte[] manifest = getManifestAsBytes(Integer.parseInt(args[1])); + ZipEntry ze = new ZipEntry("META-INF/MANIFEST.MF"); + ze.setMethod(ZipEntry.STORED); + ze.setSize(manifest.length); + ze.setCompressedSize(manifest.length); + ze.setCrc(crc.getValue()); + ze.setTime(System.currentTimeMillis()); + zos.putNextEntry(ze); + zos.write(manifest); + zos.flush(); + + // add a zero length class + ze = new ZipEntry(SOME_KLASS + ".class"); + ze.setMethod(ZipEntry.STORED); + ze.setSize(0); + ze.setCompressedSize(0); + ze.setCrc(0); + ze.setTime(System.currentTimeMillis()); + zos.putNextEntry(ze); + zos.flush(); + zos.closeEntry(); + zos.close(); + System.exit(0); + } +} From 3729356740fb70966397e582528db89a60e0824e Mon Sep 17 00:00:00 2001 From: Masayoshi Okutsu Date: Thu, 2 Oct 2008 16:49:33 +0900 Subject: [PATCH 04/23] 6734167: Calendar.readObject allows elevation of privileges Reviewed-by: peytoia --- jdk/src/share/classes/java/util/Calendar.java | 54 ++++++++++++++----- 1 file changed, 42 insertions(+), 12 deletions(-) diff --git a/jdk/src/share/classes/java/util/Calendar.java b/jdk/src/share/classes/java/util/Calendar.java index e1f65e6a885..072de9e142f 100644 --- a/jdk/src/share/classes/java/util/Calendar.java +++ b/jdk/src/share/classes/java/util/Calendar.java @@ -41,9 +41,14 @@ package java.util; import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; +import java.io.OptionalDataException; import java.io.Serializable; +import java.security.AccessControlContext; import java.security.AccessController; +import java.security.PermissionCollection; +import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; +import java.security.ProtectionDomain; import java.text.DateFormat; import java.text.DateFormatSymbols; import sun.util.BuddhistCalendar; @@ -2626,6 +2631,18 @@ public abstract class Calendar implements Serializable, Cloneable, Comparable() { + public ZoneInfo run() throws Exception { + return (ZoneInfo) input.readObject(); + } + }, + CalendarAccessControlContext.INSTANCE); + } catch (PrivilegedActionException pae) { + Exception e = pae.getException(); + if (!(e instanceof OptionalDataException)) { + if (e instanceof RuntimeException) { + throw (RuntimeException) e; + } else if (e instanceof IOException) { + throw (IOException) e; + } else if (e instanceof ClassNotFoundException) { + throw (ClassNotFoundException) e; + } + throw new RuntimeException(e); } - } catch (Exception e) { + } + if (zi != null) { + zone = zi; } // If the deserialized object has a SimpleTimeZone, try to @@ -2674,9 +2704,9 @@ public abstract class Calendar implements Serializable, Cloneable, Comparable Date: Thu, 2 Oct 2008 20:37:43 +0400 Subject: [PATCH 05/23] 6726779: ConvolveOp on USHORT raster can cause the JVM crash Reviewed-by: igor, prr --- .../native/sun/awt/medialib/awt_ImagingLib.c | 38 +++----- .../awt/image/ConvolveOp/EdgeNoOpCrash.java | 95 +++++++++++++++++++ 2 files changed, 107 insertions(+), 26 deletions(-) create mode 100644 jdk/test/java/awt/image/ConvolveOp/EdgeNoOpCrash.java diff --git a/jdk/src/share/native/sun/awt/medialib/awt_ImagingLib.c b/jdk/src/share/native/sun/awt/medialib/awt_ImagingLib.c index 681f26290b8..157827fef29 100644 --- a/jdk/src/share/native/sun/awt/medialib/awt_ImagingLib.c +++ b/jdk/src/share/native/sun/awt/medialib/awt_ImagingLib.c @@ -216,6 +216,16 @@ printMedialibError(int status) { #endif /* ! DEBUG */ +static int +getMlibEdgeHint(jint edgeHint) { + switch (edgeHint) { + case java_awt_image_ConvolveOp_EDGE_NO_OP: + return MLIB_EDGE_DST_COPY_SRC; + case java_awt_image_ConvolveOp_EDGE_ZERO_FILL: + default: + return MLIB_EDGE_DST_FILL_ZERO; + } +} /*************************************************************************** * External Functions * @@ -400,22 +410,10 @@ Java_sun_awt_image_ImagingLib_convolveBI(JNIEnv *env, jobject this, } } - if (edgeHint == java_awt_image_ConvolveOp_EDGE_NO_OP) { - int kw2 = kwidth>>1; - int kh2 = kheight>>1; - int bsize = mlib_ImageGetChannels(src)* - (mlib_ImageGetType(src) == MLIB_BYTE ? 1 : 2); - - void *dstDataP = mlib_ImageGetData(dst); - void *srcDataP = mlib_ImageGetData(src); - /* REMIND: Copy a smaller area */ - memcpy(dstDataP, srcDataP, dst->width*dst->height*bsize); - } - cmask = (1<channels)-1; status = (*sMlibFns[MLIB_CONVMxN].fptr)(dst, src, kdata, w, h, (w-1)/2, (h-1)/2, scale, cmask, - MLIB_EDGE_DST_NO_WRITE); + getMlibEdgeHint(edgeHint)); if (status != MLIB_SUCCESS) { printMedialibError(status); @@ -660,22 +658,10 @@ Java_sun_awt_image_ImagingLib_convolveRaster(JNIEnv *env, jobject this, } } - if (edgeHint == java_awt_image_ConvolveOp_EDGE_NO_OP) { - int kw2 = kwidth>>1; - int kh2 = kheight>>1; - int bsize = mlib_ImageGetChannels(src)* - (mlib_ImageGetType(src) == MLIB_BYTE ? 1 : 2); - - void *dstDataP = mlib_ImageGetData(dst); - void *srcDataP = mlib_ImageGetData(src); - /* REMIND: Copy a smaller area */ - memcpy(dstDataP, srcDataP, dst->width*dst->height*bsize); - } - cmask = (1<channels)-1; status = (*sMlibFns[MLIB_CONVMxN].fptr)(dst, src, kdata, w, h, (w-1)/2, (h-1)/2, scale, cmask, - MLIB_EDGE_DST_NO_WRITE); + getMlibEdgeHint(edgeHint)); if (status != MLIB_SUCCESS) { printMedialibError(status); diff --git a/jdk/test/java/awt/image/ConvolveOp/EdgeNoOpCrash.java b/jdk/test/java/awt/image/ConvolveOp/EdgeNoOpCrash.java new file mode 100644 index 00000000000..5e1d2eb3f66 --- /dev/null +++ b/jdk/test/java/awt/image/ConvolveOp/EdgeNoOpCrash.java @@ -0,0 +1,95 @@ +/* + * Copyright 2008 Sun Microsystems, Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, + * CA 95054 USA or visit www.sun.com if you need additional information or + * have any questions. + */ + +/* + * @test + * @bug 6726779 + * @summary Test verifies that ConvolveOp with the EDGE_NO_OP edge condition + * does not cause JVM crash if size of source raster elements is + * greather than size of the destination raster element. + * + * @run main EdgeNoOpCrash + */ +import java.awt.Point; +import java.awt.image.ConvolveOp; +import java.awt.image.DataBuffer; +import java.awt.image.ImagingOpException; +import java.awt.image.Kernel; +import java.awt.image.Raster; +import java.awt.image.WritableRaster; +import java.util.Arrays; + +public class EdgeNoOpCrash { + private static final int w = 3000; + private static final int h = 200; + + public static void main(String[] args) { + crashTest(); + } + + private static void crashTest() { + Raster src = createSrcRaster(); + WritableRaster dst = createDstRaster(); + ConvolveOp op = createConvolveOp(ConvolveOp.EDGE_NO_OP); + try { + op.filter(src, dst); + } catch (ImagingOpException e) { + /* + * The test pair of source and destination rasters + * may cause failure of the medialib convolution routine, + * so this exception is expected. + * + * The JVM crash is the only manifestation of this + * test failure. + */ + } + System.out.println("Test PASSED."); + } + + private static Raster createSrcRaster() { + WritableRaster r = Raster.createInterleavedRaster(DataBuffer.TYPE_USHORT, + w, h, 4, new Point(0, 0)); + + return r; + } + + private static WritableRaster createDstRaster() { + WritableRaster r = Raster.createInterleavedRaster(DataBuffer.TYPE_BYTE, + w, h, 4, new Point(0, 0)); + + return r; + } + + private static ConvolveOp createConvolveOp(int edgeHint) { + final int kw = 3; + final int kh = 3; + float[] kdata = new float[kw * kh]; + float v = 1f / kdata.length; + Arrays.fill(kdata, v); + + Kernel k = new Kernel(kw, kh, kdata); + ConvolveOp op = new ConvolveOp(k, edgeHint, null); + + return op; + } +} \ No newline at end of file From 01bf9872448b93153ab539f370858bf830c3fd7d Mon Sep 17 00:00:00 2001 From: Alan Bateman Date: Thu, 9 Oct 2008 21:12:56 +0100 Subject: [PATCH 06/23] 6721753: File.createTempFile produces guessable file names Reviewed-by: sherman --- jdk/src/share/classes/java/io/File.java | 54 ++++++++++++------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/jdk/src/share/classes/java/io/File.java b/jdk/src/share/classes/java/io/File.java index 7c58136a44d..8433c75d488 100644 --- a/jdk/src/share/classes/java/io/File.java +++ b/jdk/src/share/classes/java/io/File.java @@ -33,9 +33,9 @@ import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Map; import java.util.Hashtable; -import java.util.Random; import java.security.AccessController; import java.security.AccessControlException; +import java.security.SecureRandom; import sun.security.action.GetPropertyAction; @@ -1678,28 +1678,28 @@ public class File /* -- Temporary files -- */ - private static final Object tmpFileLock = new Object(); + // lazy initialization of SecureRandom and temporary file directory + private static class LazyInitialization { + static final SecureRandom random = new SecureRandom(); - private static int counter = -1; /* Protected by tmpFileLock */ + static final String temporaryDirectory = temporaryDirectory(); + static String temporaryDirectory() { + return fs.normalize( + AccessController.doPrivileged( + new GetPropertyAction("java.io.tmpdir"))); + } + } private static File generateFile(String prefix, String suffix, File dir) throws IOException { - if (counter == -1) { - counter = new Random().nextInt() & 0xffff; + long n = LazyInitialization.random.nextLong(); + if (n == Long.MIN_VALUE) { + n = 0; // corner case + } else { + n = Math.abs(n); } - counter++; - return new File(dir, prefix + Integer.toString(counter) + suffix); - } - - private static String tmpdir; /* Protected by tmpFileLock */ - - private static String getTempDir() { - if (tmpdir == null) - tmpdir = fs.normalize( - AccessController.doPrivileged( - new GetPropertyAction("java.io.tmpdir"))); - return tmpdir; + return new File(dir, prefix + Long.toString(n) + suffix); } private static boolean checkAndCreate(String filename, SecurityManager sm) @@ -1795,18 +1795,16 @@ public class File if (prefix.length() < 3) throw new IllegalArgumentException("Prefix string too short"); String s = (suffix == null) ? ".tmp" : suffix; - synchronized (tmpFileLock) { - if (directory == null) { - String tmpDir = getTempDir(); - directory = new File(tmpDir, fs.prefixLength(tmpDir)); - } - SecurityManager sm = System.getSecurityManager(); - File f; - do { - f = generateFile(prefix, s, directory); - } while (!checkAndCreate(f.getPath(), sm)); - return f; + if (directory == null) { + String tmpDir = LazyInitialization.temporaryDirectory(); + directory = new File(tmpDir, fs.prefixLength(tmpDir)); } + SecurityManager sm = System.getSecurityManager(); + File f; + do { + f = generateFile(prefix, s, directory); + } while (!checkAndCreate(f.getPath(), sm)); + return f; } /** From 92992b2e764bc12439d30b28c4161445e86db0bb Mon Sep 17 00:00:00 2001 From: Kumar Srinivasan Date: Fri, 17 Oct 2008 09:43:30 -0700 Subject: [PATCH 07/23] 6755943: Java JAR Pack200 Decompression should enforce stricter header checks Fixes a core dump when fed with a faulty pack file and related malicious take over Reviewed-by: jrose --- jdk/make/common/shared/Defs-windows.gmk | 4 + .../com/sun/java/util/jar/pack/bytes.cpp | 6 +- .../com/sun/java/util/jar/pack/defines.h | 10 +- .../com/sun/java/util/jar/pack/main.cpp | 4 +- .../com/sun/java/util/jar/pack/unpack.cpp | 24 +- .../com/sun/java/util/jar/pack/unpack.h | 4 +- .../com/sun/java/util/jar/pack/utils.cpp | 11 +- .../native/com/sun/java/util/jar/pack/utils.h | 22 +- .../tools/pack200/MemoryAllocatorTest.java | 369 ++++++++++++++++++ 9 files changed, 423 insertions(+), 31 deletions(-) create mode 100644 jdk/test/tools/pack200/MemoryAllocatorTest.java diff --git a/jdk/make/common/shared/Defs-windows.gmk b/jdk/make/common/shared/Defs-windows.gmk index 250604136bc..d829a41853e 100644 --- a/jdk/make/common/shared/Defs-windows.gmk +++ b/jdk/make/common/shared/Defs-windows.gmk @@ -539,6 +539,7 @@ else WSCRIPT :=$(call FileExists,$(_WSCRIPT1),$(_WSCRIPT2)) endif WSCRIPT:=$(call AltCheckSpaces,WSCRIPT) +WSCRIPT += -B # CSCRIPT: path to cscript.exe (used in creating install bundles) ifdef ALT_CSCRIPT @@ -561,6 +562,9 @@ else MSIVAL2 :=$(call FileExists,$(_MSIVAL2_1),$(_MSIVAL2_2)) endif MSIVAL2:=$(call AltCheckSpaces,MSIVAL2) +ifdef SKIP_MSIVAL2 + MSIVAL2 := $(ECHO) +endif # LOGOCUB: path to cub file for (used in validating install msi files) ifdef ALT_LOGOCUB diff --git a/jdk/src/share/native/com/sun/java/util/jar/pack/bytes.cpp b/jdk/src/share/native/com/sun/java/util/jar/pack/bytes.cpp index 95c3dde66e9..36843061354 100644 --- a/jdk/src/share/native/com/sun/java/util/jar/pack/bytes.cpp +++ b/jdk/src/share/native/com/sun/java/util/jar/pack/bytes.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2003 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2001-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -56,7 +56,7 @@ void bytes::realloc(size_t len_) { return; } byte* oldptr = ptr; - ptr = (byte*)::realloc(ptr, len_+1); + ptr = (len_ >= PSIZE_MAX) ? null : (byte*)::realloc(ptr, len_+1); if (ptr != null) { mtrace('r', oldptr, 0); mtrace('m', ptr, len_+1); @@ -126,7 +126,7 @@ const char* bytes::string() { // Make sure there are 'o' bytes beyond the fill pointer, // advance the fill pointer, and return the old fill pointer. byte* fillbytes::grow(size_t s) { - size_t nlen = b.len+s; + size_t nlen = add_size(b.len, s); if (nlen <= allocated) { b.len = nlen; return limit()-s; diff --git a/jdk/src/share/native/com/sun/java/util/jar/pack/defines.h b/jdk/src/share/native/com/sun/java/util/jar/pack/defines.h index 19f7567e7bc..f780f247b2a 100644 --- a/jdk/src/share/native/com/sun/java/util/jar/pack/defines.h +++ b/jdk/src/share/native/com/sun/java/util/jar/pack/defines.h @@ -1,5 +1,5 @@ /* - * Copyright 2001-2004 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2001-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -47,11 +47,13 @@ #define NOT_PRODUCT(xxx) #define assert(p) (0) #define printcr false && +#define VERSION_STRING "%s version %s\n" #else #define IF_PRODUCT(xxx) #define NOT_PRODUCT(xxx) xxx #define assert(p) ((p) || (assert_failed(#p), 1)) #define printcr u->verbose && u->printcr_if_verbose +#define VERSION_STRING "%s version non-product %s\n" extern "C" void breakpoint(); extern void assert_failed(const char*); #define BREAK (breakpoint()) @@ -79,9 +81,9 @@ extern void assert_failed(const char*); #define lengthof(array) (sizeof(array)/sizeof(array[0])) -#define NEW(T, n) (T*) must_malloc(sizeof(T)*(n)) -#define U_NEW(T, n) (T*) u->alloc(sizeof(T)*(n)) -#define T_NEW(T, n) (T*) u->temp_alloc(sizeof(T)*(n)) +#define NEW(T, n) (T*) must_malloc(scale_size(n, sizeof(T))) +#define U_NEW(T, n) (T*) u->alloc(scale_size(n, sizeof(T))) +#define T_NEW(T, n) (T*) u->temp_alloc(scale_size(n, sizeof(T))) // bytes and byte arrays diff --git a/jdk/src/share/native/com/sun/java/util/jar/pack/main.cpp b/jdk/src/share/native/com/sun/java/util/jar/pack/main.cpp index 9e216cd19c7..234a6009da5 100644 --- a/jdk/src/share/native/com/sun/java/util/jar/pack/main.cpp +++ b/jdk/src/share/native/com/sun/java/util/jar/pack/main.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2003-2005 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2003-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -300,7 +300,7 @@ int unpacker::run(int argc, char **argv) { case 'J': argp += 1; break; // skip ignored -Jxxx parameter case 'V': - fprintf(u.errstrm, "%s version %s\n", nbasename(argv[0]), sccsver); + fprintf(u.errstrm, VERSION_STRING, nbasename(argv[0]), sccsver); exit(0); case 'h': diff --git a/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.cpp b/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.cpp index 6480da70021..08fbbc2aa4a 100644 --- a/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.cpp +++ b/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2005 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2001-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -618,18 +618,17 @@ void unpacker::read_file_header() { if ((archive_options & AO_HAVE_FILE_HEADERS) != 0) { uint hi = hdr.getInt(); uint lo = hdr.getInt(); - archive_size = band::makeLong(hi, lo); + julong x = band::makeLong(hi, lo); + archive_size = (size_t) x; + if (archive_size != x) { + // Silly size specified; force overflow. + archive_size = PSIZE_MAX+1; + } hdrVals += 2; } else { hdrValsSkipped += 2; } - if (archive_size != (size_t)archive_size) { - // Silly size specified. - abort("archive too large"); - return; - } - // Now we can size the whole archive. // Read everything else into a mega-buffer. rp = hdr.rp; @@ -643,8 +642,8 @@ void unpacker::read_file_header() { abort("EOF reading fixed input buffer"); return; } - } else if (archive_size > 0) { - input.set(U_NEW(byte, (size_t) header_size_0 + archive_size + C_SLOP), + } else if (archive_size != 0) { + input.set(U_NEW(byte, add_size(header_size_0, archive_size, C_SLOP)), (size_t) header_size_0 + archive_size); assert(input.limit()[0] == 0); // Move all the bytes we read initially into the real buffer. @@ -654,7 +653,6 @@ void unpacker::read_file_header() { } else { // It's more complicated and painful. // A zero archive_size means that we must read until EOF. - assert(archive_size == 0); input.init(CHUNK*2); CHECK; input.b.len = input.allocated; @@ -664,7 +662,7 @@ void unpacker::read_file_header() { rplimit += header_size; while (ensure_input(input.limit() - rp)) { size_t dataSoFar = input_remaining(); - size_t nextSize = dataSoFar + CHUNK; + size_t nextSize = add_size(dataSoFar, CHUNK); input.ensureSize(nextSize); CHECK; input.b.len = input.allocated; @@ -949,10 +947,12 @@ void unpacker::read_Utf8_values(entry* cpMap, int len) { // First band: Read lengths of shared prefixes. if (len > PREFIX_SKIP_2) cp_Utf8_prefix.readData(len - PREFIX_SKIP_2); + NOT_PRODUCT(else cp_Utf8_prefix.readData(0)); // for asserts // Second band: Read lengths of unshared suffixes: if (len > SUFFIX_SKIP_1) cp_Utf8_suffix.readData(len - SUFFIX_SKIP_1); + NOT_PRODUCT(else cp_Utf8_suffix.readData(0)); // for asserts bytes* allsuffixes = T_NEW(bytes, len); CHECK; diff --git a/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.h b/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.h index 7ab8e07b2ac..03e2edadcd3 100644 --- a/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.h +++ b/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.h @@ -1,5 +1,5 @@ /* - * Copyright 2002-2005 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2002-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -204,7 +204,7 @@ struct unpacker { // archive header fields int magic, minver, majver; - julong archive_size; + size_t archive_size; int archive_next_count, archive_options, archive_modtime; int band_headers_size; int file_count, attr_definition_count, ic_count, class_count; diff --git a/jdk/src/share/native/com/sun/java/util/jar/pack/utils.cpp b/jdk/src/share/native/com/sun/java/util/jar/pack/utils.cpp index 4f45c84b524..8a88dcd961e 100644 --- a/jdk/src/share/native/com/sun/java/util/jar/pack/utils.cpp +++ b/jdk/src/share/native/com/sun/java/util/jar/pack/utils.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2004 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2001-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -46,14 +46,13 @@ #include "unpack.h" -void* must_malloc(int size) { - int msize = size; - assert(size >= 0); +void* must_malloc(size_t size) { + size_t msize = size; #ifdef USE_MTRACE - if (msize < sizeof(int)) + if (msize >= 0 && msize < sizeof(int)) msize = sizeof(int); // see 0xbaadf00d below #endif - void* ptr = malloc(msize); + void* ptr = (msize > PSIZE_MAX) ? null : malloc(msize); if (ptr != null) { memset(ptr, 0, size); } else { diff --git a/jdk/src/share/native/com/sun/java/util/jar/pack/utils.h b/jdk/src/share/native/com/sun/java/util/jar/pack/utils.h index 6176634bcaf..cc211400eac 100644 --- a/jdk/src/share/native/com/sun/java/util/jar/pack/utils.h +++ b/jdk/src/share/native/com/sun/java/util/jar/pack/utils.h @@ -1,5 +1,5 @@ /* - * Copyright 2001-2003 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2001-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -25,13 +25,31 @@ //Definitions of our util functions -void* must_malloc(int size); +void* must_malloc(size_t size); #ifndef USE_MTRACE #define mtrace(c, ptr, size) (0) #else void mtrace(char c, void* ptr, size_t size); #endif +// overflow management +#define OVERFLOW ((size_t)-1) +#define PSIZE_MAX (OVERFLOW/2) /* normal size limit */ + +inline size_t scale_size(size_t size, size_t scale) { + return (size > PSIZE_MAX / scale) ? OVERFLOW : size * scale; +} + +inline size_t add_size(size_t size1, size_t size2) { + return ((size1 | size2 | (size1 + size2)) > PSIZE_MAX) + ? OVERFLOW + : size1 + size2; +} + +inline size_t add_size(size_t size1, size_t size2, int size3) { + return add_size(add_size(size1, size2), size3); +} + // These may be expensive, because they have to go via Java TSD, // if the optional u argument is missing. struct unpacker; diff --git a/jdk/test/tools/pack200/MemoryAllocatorTest.java b/jdk/test/tools/pack200/MemoryAllocatorTest.java new file mode 100644 index 00000000000..6015a4e470d --- /dev/null +++ b/jdk/test/tools/pack200/MemoryAllocatorTest.java @@ -0,0 +1,369 @@ +/* + * Copyright 2008 Sun Microsystems, Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, + * CA 95054 USA or visit www.sun.com if you need additional information or + * have any questions. + */ + +/* + * @test + * @bug 6755943 + * @summary Checks any memory overruns in archive length. + * @run main/timeout=1200 MemoryAllocatorTest + */ +import java.io.BufferedReader; +import java.io.DataOutputStream; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.io.RandomAccessFile; +import java.nio.MappedByteBuffer; +import java.nio.channels.FileChannel; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +public class MemoryAllocatorTest { + + /* + * The smallest possible pack file with 1 empty resource + */ + static int[] magic = { + 0xCA, 0xFE, 0xD0, 0x0D + }; + static int[] version_info = { + 0x07, // minor + 0x96 // major + }; + static int[] option = { + 0x10 + }; + static int[] size_hi = { + 0x00 + }; + static int[] size_lo_ulong = { + 0xFF, 0xFC, 0xFC, 0xFC, 0xFC // ULONG_MAX 0xFFFFFFFF + }; + static int[] size_lo_correct = { + 0x17 + }; + static int[] data = { + 0x00, 0xEC, 0xDA, 0xDE, 0xF8, 0x45, 0x01, 0x02, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x01, 0x31, 0x01, 0x00 + }; + // End of pack file data + + static final String JAVA_HOME = System.getProperty("java.home"); + + static final boolean debug = Boolean.getBoolean("MemoryAllocatorTest.Debug"); + static final boolean WINDOWS = System.getProperty("os.name").startsWith("Windows"); + static final boolean LINUX = System.getProperty("os.name").startsWith("Linux"); + static final boolean SIXTYFOUR_BIT = System.getProperty("sun.arch.data.model", "32").equals("64"); + static final private int EXPECTED_EXIT_CODE = (WINDOWS) ? -1 : 255; + + static int testExitValue = 0; + + static byte[] bytes(int[] a) { + byte[] b = new byte[a.length]; + for (int i = 0; i < b.length; i++) { + b[i] = (byte) a[i]; + } + return b; + } + + static void createPackFile(boolean good, File packFile) throws IOException { + FileOutputStream fos = new FileOutputStream(packFile); + fos.write(bytes(magic)); + fos.write(bytes(version_info)); + fos.write(bytes(option)); + fos.write(bytes(size_hi)); + if (good) { + fos.write(bytes(size_lo_correct)); + } else { + fos.write(bytes(size_lo_ulong)); + } + fos.write(bytes(data)); + } + + /* + * This method modifies the LSB of the size_lo for various wicked + * values between MAXINT-0x3F and MAXINT. + */ + static int modifyPackFile(File packFile) throws IOException { + RandomAccessFile raf = new RandomAccessFile(packFile, "rws"); + long len = packFile.length(); + FileChannel fc = raf.getChannel(); + MappedByteBuffer bb = fc.map(FileChannel.MapMode.READ_WRITE, 0, len); + int pos = magic.length + version_info.length + option.length + + size_hi.length; + byte value = bb.get(pos); + value--; + bb.position(pos); + bb.put(value); + bb.force(); + fc.truncate(len); + fc.close(); + return value & 0xFF; + } + + static String getUnpack200Cmd() throws Exception { + File binDir = new File(JAVA_HOME, "bin"); + File unpack200File = WINDOWS + ? new File(binDir, "unpack200.exe") + : new File(binDir, "unpack200"); + + String cmd = unpack200File.getAbsolutePath(); + if (!unpack200File.canExecute()) { + throw new Exception("please check" + + cmd + " exists and is executable"); + } + return cmd; + } + + static TestResult runUnpacker(File packFile) throws Exception { + if (!packFile.exists()) { + throw new Exception("please check" + packFile + " exists"); + } + ArrayList alist = new ArrayList(); + ProcessBuilder pb = new ProcessBuilder(getUnpack200Cmd(), + packFile.getName(), "testout.jar"); + Map env = pb.environment(); + pb.directory(new File(".")); + int retval = 0; + try { + pb.redirectErrorStream(true); + Process p = pb.start(); + BufferedReader rd = new BufferedReader( + new InputStreamReader(p.getInputStream()), 8192); + String in = rd.readLine(); + while (in != null) { + alist.add(in); + System.out.println(in); + in = rd.readLine(); + } + retval = p.waitFor(); + p.destroy(); + } catch (Exception ex) { + ex.printStackTrace(); + throw new RuntimeException(ex.getMessage()); + } + return new TestResult("", retval, alist); + } + + /* + * The debug version builds of unpack200 call abort(3) which might set + * an unexpected return value, therefore this test is to determine + * if we are using a product or non-product build and check the + * return value appropriately. + */ + static boolean isNonProductVersion() throws Exception { + ArrayList alist = new ArrayList(); + ProcessBuilder pb = new ProcessBuilder(getUnpack200Cmd(), "--version"); + Map env = pb.environment(); + pb.directory(new File(".")); + int retval = 0; + try { + pb.redirectErrorStream(true); + Process p = pb.start(); + BufferedReader rd = new BufferedReader( + new InputStreamReader(p.getInputStream()), 8192); + String in = rd.readLine(); + while (in != null) { + alist.add(in); + System.out.println(in); + in = rd.readLine(); + } + retval = p.waitFor(); + p.destroy(); + } catch (Exception ex) { + ex.printStackTrace(); + throw new RuntimeException(ex.getMessage()); + } + for (String x : alist) { + if (x.contains("non-product")) { + return true; + } + } + return false; + } + + /** + * @param args the command line arguments + * @throws java.lang.Exception + */ + public static void main(String[] args) throws Exception { + + File packFile = new File("tiny.pack"); + boolean isNPVersion = isNonProductVersion(); + + // Create a good pack file and test if everything is ok + createPackFile(true, packFile); + TestResult tr = runUnpacker(packFile); + tr.setDescription("a good pack file"); + tr.checkPositive(); + tr.isOK(); + System.out.println(tr); + + /* + * jprt systems on windows and linux seem to have abundant memory + * therefore can take a very long time to run, and even if it does + * the error message is not accurate for us to discern if the test + * passess successfully. + */ + if (SIXTYFOUR_BIT && (LINUX || WINDOWS)) { + System.out.println("Warning: Windows/Linux 64bit tests passes vacuously"); + return; + } + + /* + * debug builds call abort, the exit code under these conditions + * are not really relevant. + */ + if (isNPVersion) { + System.out.println("Warning: non-product build: exit values not checked"); + } + + // create a bad pack file + createPackFile(false, packFile); + tr = runUnpacker(packFile); + tr.setDescription("a wicked pack file"); + tr.contains("Native allocation failed"); + if(!isNPVersion) { + tr.checkValue(EXPECTED_EXIT_CODE); + } + System.out.println(tr); + int value = modifyPackFile(packFile); + tr.setDescription("value=" + value); + + // continue creating bad pack files by modifying the specimen pack file. + while (value >= 0xc0) { + tr = runUnpacker(packFile); + tr.contains("Native allocation failed"); + if (!isNPVersion) { + tr.checkValue(EXPECTED_EXIT_CODE); + } + tr.setDescription("wicked value=0x" + + Integer.toHexString(value & 0xFF)); + System.out.println(tr); + value = modifyPackFile(packFile); + } + if (testExitValue != 0) { + throw new Exception("Pack200 archive length tests(" + + testExitValue + ") failed "); + } else { + System.out.println("All tests pass"); + } + } + + /* + * A class to encapsulate the test results and stuff, with some ease + * of use methods to check the test results. + */ + static class TestResult { + + StringBuilder status; + int exitValue; + List testOutput; + String description; + + public TestResult(String str, int rv, List oList) { + status = new StringBuilder(str); + exitValue = rv; + testOutput = oList; + } + + void setDescription(String description) { + this.description = description; + } + + void checkValue(int value) { + if (exitValue != value) { + status = + status.append(" Error: test expected exit value " + + value + "got " + exitValue); + testExitValue++; + } + } + + void checkNegative() { + if (exitValue == 0) { + status = status.append( + " Error: test did not expect 0 exit value"); + + testExitValue++; + } + } + + void checkPositive() { + if (exitValue != 0) { + status = status.append( + " Error: test did not return 0 exit value"); + testExitValue++; + } + } + + boolean isOK() { + return exitValue == 0; + } + + boolean isZeroOutput() { + if (!testOutput.isEmpty()) { + status = status.append(" Error: No message from cmd please"); + testExitValue++; + return false; + } + return true; + } + + boolean isNotZeroOutput() { + if (testOutput.isEmpty()) { + status = status.append(" Error: Missing message"); + testExitValue++; + return false; + } + return true; + } + + public String toString() { + if (debug) { + for (String x : testOutput) { + status = status.append(x + "\n"); + } + } + if (description != null) { + status.insert(0, description); + } + return status.append("\nexitValue = " + exitValue).toString(); + } + + boolean contains(String str) { + for (String x : testOutput) { + if (x.contains(str)) { + return true; + } + } + status = status.append(" Error: string <" + str + "> not found "); + testExitValue++; + return false; + } + } +} From b44236abdf1aea0dbd9fdcf04fe621c5dd7117e3 Mon Sep 17 00:00:00 2001 From: Andrew Brygin Date: Wed, 3 Dec 2008 13:34:50 +0300 Subject: [PATCH 08/23] 6766136: corrupted gif image may cause crash in java splashscreen library Reviewed-by: prr, art --- .../awt/splashscreen/splashscreen_gfx_impl.h | 2 +- .../sun/awt/splashscreen/splashscreen_gif.c | 92 +++++++++++++++---- 2 files changed, 76 insertions(+), 18 deletions(-) diff --git a/jdk/src/share/native/sun/awt/splashscreen/splashscreen_gfx_impl.h b/jdk/src/share/native/sun/awt/splashscreen/splashscreen_gfx_impl.h index 1c55702fe73..32f7d4e2e47 100644 --- a/jdk/src/share/native/sun/awt/splashscreen/splashscreen_gfx_impl.h +++ b/jdk/src/share/native/sun/awt/splashscreen/splashscreen_gfx_impl.h @@ -31,7 +31,7 @@ /* here come some very simple macros */ /* advance a pointer p by sizeof(type)*n bytes */ -#define INCPN(type,p,n) ((p) = (type*)(p)+n) +#define INCPN(type,p,n) ((p) = (type*)(p)+(n)) /* advance a pointer by sizeof(type) */ #define INCP(type,p) INCPN(type,(p),1) diff --git a/jdk/src/share/native/sun/awt/splashscreen/splashscreen_gif.c b/jdk/src/share/native/sun/awt/splashscreen/splashscreen_gif.c index f6d9acb0c36..71bc3a3b39d 100644 --- a/jdk/src/share/native/sun/awt/splashscreen/splashscreen_gif.c +++ b/jdk/src/share/native/sun/awt/splashscreen/splashscreen_gif.c @@ -53,6 +53,10 @@ static const char szNetscape20ext[11] = "NETSCAPE2.0"; // convert libungif samples to our ones #define MAKE_QUAD_GIF(c,a) MAKE_QUAD((c).Red, (c).Green, (c).Blue, (a)) +#define SAFE_TO_ALLOC(c, sz) \ + (((c) > 0) && ((sz) > 0) && \ + ((0xffffffffu / ((unsigned int)(c))) > (unsigned int)(sz))) + /* stdio FILE* and memory input functions for libungif */ int SplashStreamGifInputFunc(GifFileType * gif, GifByteType * buf, int n) @@ -62,6 +66,15 @@ SplashStreamGifInputFunc(GifFileType * gif, GifByteType * buf, int n) return rc; } +/* These macro help to ensure that we only take part of frame that fits into + logical screen. */ + +/* Ensure that p belongs to [pmin, pmax) interval. Returns fixed point (if fix is needed) */ +#define FIX_POINT(p, pmin, pmax) ( ((p) < (pmin)) ? (pmin) : (((p) > (pmax)) ? (pmax) : (p))) +/* Ensures that line starting at point p does not exceed boundary pmax. + Returns fixed length (if fix is needed) */ +#define FIX_LENGTH(p, len, pmax) ( ((p) + (len)) > (pmax) ? ((pmax) - (p)) : (len)) + int SplashDecodeGif(Splash * splash, GifFileType * gif) { @@ -70,6 +83,7 @@ SplashDecodeGif(Splash * splash, GifFileType * gif) byte_t *pBitmapBits, *pOldBitmapBits; int i, j; int imageIndex; + int cx, cy, cw, ch; /* clamped coordinates */ const int interlacedOffset[] = { 0, 4, 2, 1, 0 }; /* The way Interlaced image should. */ const int interlacedJumps[] = { 8, 8, 4, 2, 1 }; /* be read - offsets and jumps... */ @@ -79,14 +93,31 @@ SplashDecodeGif(Splash * splash, GifFileType * gif) SplashCleanup(splash); + if (!SAFE_TO_ALLOC(gif->SWidth, splash->imageFormat.depthBytes)) { + return 0; + } stride = gif->SWidth * splash->imageFormat.depthBytes; if (splash->byteAlignment > 1) stride = (stride + splash->byteAlignment - 1) & ~(splash->byteAlignment - 1); + if (!SAFE_TO_ALLOC(gif->SHeight, stride)) { + return 0; + } + + if (!SAFE_TO_ALLOC(gif->ImageCount, sizeof(SplashImage*))) { + return 0; + } bufferSize = stride * gif->SHeight; pBitmapBits = (byte_t *) malloc(bufferSize); + if (!pBitmapBits) { + return 0; + } pOldBitmapBits = (byte_t *) malloc(bufferSize); + if (!pOldBitmapBits) { + free(pBitmapBits); + return 0; + } memset(pBitmapBits, 0, bufferSize); splash->width = gif->SWidth; @@ -94,6 +125,11 @@ SplashDecodeGif(Splash * splash, GifFileType * gif) splash->frameCount = gif->ImageCount; splash->frames = (SplashImage *) malloc(sizeof(SplashImage) * gif->ImageCount); + if (!splash->frames) { + free(pBitmapBits); + free(pOldBitmapBits); + return 0; + } memset(splash->frames, 0, sizeof(SplashImage) * gif->ImageCount); splash->loopCount = 1; @@ -109,6 +145,11 @@ SplashDecodeGif(Splash * splash, GifFileType * gif) int colorCount = 0; rgbquad_t colorMapBuf[SPLASH_COLOR_MAP_SIZE]; + cx = FIX_POINT(desc->Left, 0, gif->SWidth); + cy = FIX_POINT(desc->Top, 0, gif->SHeight); + cw = FIX_LENGTH(desc->Left, desc->Width, gif->SWidth); + ch = FIX_LENGTH(desc->Top, desc->Height, gif->SHeight); + if (colorMap) { if (colorMap->ColorCount <= SPLASH_COLOR_MAP_SIZE) { colorCount = colorMap->ColorCount; @@ -195,13 +236,22 @@ SplashDecodeGif(Splash * splash, GifFileType * gif) for (; pass < npass; ++pass) { int jump = interlacedJumps[pass]; int ofs = interlacedOffset[pass]; - int numLines = (desc->Height + jump - 1 - ofs) / jump; + /* Number of source lines for current pass */ + int numPassLines = (desc->Height + jump - ofs - 1) / jump; + /* Number of lines that fits to dest buffer */ + int numLines = (ch + jump - ofs - 1) / jump; initRect(&srcRect, 0, 0, desc->Width, numLines, 1, desc->Width, pSrc, &srcFormat); - initRect(&dstRect, desc->Left, desc->Top + ofs, desc->Width, - numLines, jump, stride, pBitmapBits, &splash->imageFormat); - pSrc += convertRect(&srcRect, &dstRect, CVT_ALPHATEST); + + if (numLines > 0) { + initRect(&dstRect, cx, cy + ofs, cw, + numLines , jump, stride, pBitmapBits, &splash->imageFormat); + + pSrc += convertRect(&srcRect, &dstRect, CVT_ALPHATEST); + } + // skip extra source data + pSrc += (numPassLines - numLines) * srcRect.stride; } } @@ -209,6 +259,12 @@ SplashDecodeGif(Splash * splash, GifFileType * gif) splash->frames[imageIndex].bitmapBits = (rgbquad_t *) malloc(bufferSize); + if (!splash->frames[imageIndex].bitmapBits) { + free(pBitmapBits); + free(pOldBitmapBits); + /* Assuming that callee will take care of splash frames we have already allocated */ + return 0; + } memcpy(splash->frames[imageIndex].bitmapBits, pBitmapBits, bufferSize); SplashInitFrameShape(splash, imageIndex); @@ -224,27 +280,29 @@ SplashDecodeGif(Splash * splash, GifFileType * gif) { ImageRect dstRect; rgbquad_t fillColor = 0; // 0 is transparent - if (transparentColor < 0) { + + if (transparentColor > 0) { fillColor= MAKE_QUAD_GIF( colorMap->Colors[gif->SBackGroundColor], 0xff); } - initRect(&dstRect, desc->Left, desc->Top, - desc->Width, desc->Height, 1, stride, - pBitmapBits, &splash->imageFormat); + initRect(&dstRect, + cx, cy, cw, ch, + 1, stride, + pBitmapBits, &splash->imageFormat); fillRect(fillColor, &dstRect); } break; case GIF_DISPOSE_RESTORE: { - - int lineSize = desc->Width * splash->imageFormat.depthBytes; - - for (j = 0; j < desc->Height; j++) { - int lineIndex = stride * (j + desc->Top) + - desc->Left * splash->imageFormat.depthBytes; - - memcpy(pBitmapBits + lineIndex, pOldBitmapBits + lineIndex, - lineSize); + int lineSize = cw * splash->imageFormat.depthBytes; + if (lineSize > 0) { + int lineOffset = cx * splash->imageFormat.depthBytes; + int lineIndex = cy * stride + lineOffset; + for (j=0; j Date: Wed, 24 Dec 2008 15:48:59 -0800 Subject: [PATCH 09/23] 6652463: MediaSize constructors allow to redefine the mapping of standard MediaSizeName values Reviewed-by: igor, jgodinez --- .../print/attribute/standard/MediaSize.java | 12 +++-- .../print/attribute/MediaMappingsTest.java | 48 +++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 jdk/test/javax/print/attribute/MediaMappingsTest.java diff --git a/jdk/src/share/classes/javax/print/attribute/standard/MediaSize.java b/jdk/src/share/classes/javax/print/attribute/standard/MediaSize.java index 55cd1aed923..3b6262d2387 100644 --- a/jdk/src/share/classes/javax/print/attribute/standard/MediaSize.java +++ b/jdk/src/share/classes/javax/print/attribute/standard/MediaSize.java @@ -123,8 +123,10 @@ public class MediaSize extends Size2DSyntax implements Attribute { if (x > y) { throw new IllegalArgumentException("X dimension > Y dimension"); } - mediaName = media; - mediaMap.put(mediaName, this); + if (media != null && mediaMap.get(media) == null) { + mediaName = media; + mediaMap.put(mediaName, this); + } sizeVector.add(this); } @@ -147,8 +149,10 @@ public class MediaSize extends Size2DSyntax implements Attribute { if (x > y) { throw new IllegalArgumentException("X dimension > Y dimension"); } - mediaName = media; - mediaMap.put(mediaName, this); + if (media != null && mediaMap.get(media) == null) { + mediaName = media; + mediaMap.put(mediaName, this); + } sizeVector.add(this); } diff --git a/jdk/test/javax/print/attribute/MediaMappingsTest.java b/jdk/test/javax/print/attribute/MediaMappingsTest.java new file mode 100644 index 00000000000..16110d8a49e --- /dev/null +++ b/jdk/test/javax/print/attribute/MediaMappingsTest.java @@ -0,0 +1,48 @@ +/* + * Copyright 2008 Sun Microsystems, Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, + * CA 95054 USA or visit www.sun.com if you need additional information or + * have any questions. + */ + +/* + * @test + * @bug 6652463 + * @summary Verify media size-> media mappings can't be altered + * @run main MediaMappingsTest +*/ + +import javax.print.attribute.standard.MediaSize; +import javax.print.attribute.standard.MediaSizeName; + +public class MediaMappingsTest { + + public static void main(String args[]) { + MediaSize sizeA = MediaSize.getMediaSizeForName(MediaSizeName.A); + new MediaSize(1.0f, 2.0f, MediaSize.MM, MediaSizeName.A); + if (!sizeA.equals(MediaSize.getMediaSizeForName(MediaSizeName.A))) { + throw new RuntimeException("mapping changed"); + } + MediaSize sizeB = MediaSize.getMediaSizeForName(MediaSizeName.B); + new MediaSize(1, 2, MediaSize.MM, MediaSizeName.B); + if (!sizeB.equals(MediaSize.getMediaSizeForName(MediaSizeName.B))) { + throw new RuntimeException("mapping changed"); + } + } +} From e33cec202fee549a07071d2472b26bc4806908d9 Mon Sep 17 00:00:00 2001 From: Weijun Wang Date: Tue, 30 Dec 2008 10:42:45 +0800 Subject: [PATCH 10/23] 6717680: LdapCtx does not close the connection if initialization fails Reviewed-by: vinnie, xuelei --- jdk/src/share/classes/com/sun/jndi/ldap/LdapCtx.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/jdk/src/share/classes/com/sun/jndi/ldap/LdapCtx.java b/jdk/src/share/classes/com/sun/jndi/ldap/LdapCtx.java index bec6fd0e761..5f9462c5931 100644 --- a/jdk/src/share/classes/com/sun/jndi/ldap/LdapCtx.java +++ b/jdk/src/share/classes/com/sun/jndi/ldap/LdapCtx.java @@ -302,7 +302,16 @@ final public class LdapCtx extends ComponentDirContext schemaTrees = new Hashtable(11, 0.75f); initEnv(); - connect(false); + try { + connect(false); + } catch (NamingException e) { + try { + close(); + } catch (Exception e2) { + // Nothing + } + throw e; + } } LdapCtx(LdapCtx existing, String newDN) throws NamingException { From c357cbe4e01b7eb44b24191232e0fa099796bb73 Mon Sep 17 00:00:00 2001 From: Phil Race Date: Mon, 5 Jan 2009 11:28:43 -0800 Subject: [PATCH 11/23] 6632886: Font.createFont can be persuaded to leak temporary files 6522586: Enforce limits on Font creation 6652929: Font.createFont(int,File) trusts File.getPath Reviewed-by: igor --- jdk/src/share/classes/java/awt/Font.java | 177 ++++++++++++------ .../classes/sun/font/CreatedFontTracker.java | 54 ++++++ jdk/src/share/classes/sun/font/FileFont.java | 17 +- .../share/classes/sun/font/FontManager.java | 11 +- jdk/test/java/awt/FontClass/CreateFont/A.ttf | Bin 0 -> 2348 bytes .../awt/FontClass/CreateFont/BigFont.java | 139 ++++++++++++++ .../awt/FontClass/CreateFont/DeleteFont.java | 83 ++++++++ .../awt/FontClass/CreateFont/DeleteFont.sh | 66 +++++++ .../awt/FontClass/CreateFont/bigfont.html | 48 +++++ .../CreateFont/fileaccess/FontFile.java | 83 ++++++++ 10 files changed, 616 insertions(+), 62 deletions(-) create mode 100644 jdk/src/share/classes/sun/font/CreatedFontTracker.java create mode 100644 jdk/test/java/awt/FontClass/CreateFont/A.ttf create mode 100644 jdk/test/java/awt/FontClass/CreateFont/BigFont.java create mode 100644 jdk/test/java/awt/FontClass/CreateFont/DeleteFont.java create mode 100644 jdk/test/java/awt/FontClass/CreateFont/DeleteFont.sh create mode 100644 jdk/test/java/awt/FontClass/CreateFont/bigfont.html create mode 100644 jdk/test/java/awt/FontClass/CreateFont/fileaccess/FontFile.java diff --git a/jdk/src/share/classes/java/awt/Font.java b/jdk/src/share/classes/java/awt/Font.java index 8dc5938d411..3ed40325b04 100644 --- a/jdk/src/share/classes/java/awt/Font.java +++ b/jdk/src/share/classes/java/awt/Font.java @@ -37,6 +37,8 @@ import java.awt.geom.Rectangle2D; import java.awt.peer.FontPeer; import java.io.*; import java.lang.ref.SoftReference; +import java.security.AccessController; +import java.security.PrivilegedExceptionAction; import java.text.AttributedCharacterIterator.Attribute; import java.text.CharacterIterator; import java.text.StringCharacterIterator; @@ -51,6 +53,7 @@ import sun.font.AttributeMap; import sun.font.AttributeValues; import sun.font.EAttribute; import sun.font.CompositeFont; +import sun.font.CreatedFontTracker; import sun.font.Font2D; import sun.font.Font2DHandle; import sun.font.FontManager; @@ -575,14 +578,16 @@ public class Font implements java.io.Serializable } /* used to implement Font.createFont */ - private Font(File fontFile, int fontFormat, boolean isCopy) + private Font(File fontFile, int fontFormat, + boolean isCopy, CreatedFontTracker tracker) throws FontFormatException { this.createdFont = true; /* Font2D instances created by this method track their font file * so that when the Font2D is GC'd it can also remove the file. */ this.font2DHandle = - FontManager.createFont2D(fontFile, fontFormat, isCopy).handle; + FontManager.createFont2D(fontFile, fontFormat, + isCopy, tracker).handle; this.name = this.font2DHandle.font2D.getFontName(Locale.getDefault()); this.style = Font.PLAIN; this.size = 1; @@ -787,6 +792,29 @@ public class Font implements java.io.Serializable return new Font(attributes); } + /** + * Used with the byte count tracker for fonts created from streams. + * If a thread can create temp files anyway, no point in counting + * font bytes. + */ + private static boolean hasTempPermission() { + + if (System.getSecurityManager() == null) { + return true; + } + File f = null; + boolean hasPerm = false; + try { + f = File.createTempFile("+~JT", ".tmp", null); + f.delete(); + f = null; + hasPerm = true; + } catch (Throwable t) { + /* inc. any kind of SecurityException */ + } + return hasPerm; + } + /** * Returns a new Font using the specified font type * and input data. The new Font is @@ -822,58 +850,96 @@ public class Font implements java.io.Serializable fontFormat != Font.TYPE1_FONT) { throw new IllegalArgumentException ("font format not recognized"); } - final InputStream fStream = fontStream; - Object ret = java.security.AccessController.doPrivileged( - new java.security.PrivilegedAction() { - public Object run() { - File tFile = null; - FileOutputStream outStream = null; - try { - tFile = File.createTempFile("+~JF", ".tmp", null); - /* Temp file deleted by font shutdown hook */ - BufferedInputStream inStream = - new BufferedInputStream(fStream); - outStream = new FileOutputStream(tFile); - int bytesRead = 0; - int bufSize = 8192; - byte [] buf = new byte[bufSize]; - while (bytesRead != -1) { - try { - bytesRead = inStream.read(buf, 0, bufSize); - } catch (Throwable t) { - throw new IOException(); - } - if (bytesRead != -1) { - outStream.write(buf, 0, bytesRead); - } - } - /* don't close the input stream */ - outStream.close(); - } catch (IOException e) { - if (outStream != null) { - try { - outStream.close(); - } catch (Exception e1) { - } - } - if (tFile != null) { - try { - tFile.delete(); - } catch (Exception e2) { - } - } - return e; - } - return tFile; - } - }); + boolean copiedFontData = false; - if (ret instanceof File) { - return new Font((File)ret, fontFormat, true); - } else if (ret instanceof IOException) { - throw (IOException)ret; - } else { - throw new FontFormatException("Couldn't access font stream"); + try { + final File tFile = AccessController.doPrivileged( + new PrivilegedExceptionAction() { + public File run() throws IOException { + return File.createTempFile("+~JF", ".tmp", null); + } + } + ); + + int totalSize = 0; + CreatedFontTracker tracker = null; + try { + final OutputStream outStream = + AccessController.doPrivileged( + new PrivilegedExceptionAction() { + public OutputStream run() throws IOException { + return new FileOutputStream(tFile); + } + } + ); + if (!hasTempPermission()) { + tracker = CreatedFontTracker.getTracker(); + } + try { + byte[] buf = new byte[8192]; + for (;;) { + int bytesRead = fontStream.read(buf); + if (bytesRead < 0) { + break; + } + if (tracker != null) { + if (totalSize+bytesRead > tracker.MAX_FILE_SIZE) { + throw new IOException("File too big."); + } + if (totalSize+tracker.getNumBytes() > + tracker.MAX_TOTAL_BYTES) + { + throw new IOException("Total files too big."); + } + totalSize += bytesRead; + tracker.addBytes(bytesRead); + } + outStream.write(buf, 0, bytesRead); + } + /* don't close the input stream */ + } finally { + outStream.close(); + } + /* After all references to a Font2D are dropped, the file + * will be removed. To support long-lived AppContexts, + * we need to then decrement the byte count by the size + * of the file. + * If the data isn't a valid font, the implementation will + * delete the tmp file and decrement the byte count + * in the tracker object before returning from the + * constructor, so we can set 'copiedFontData' to true here + * without waiting for the results of that constructor. + */ + copiedFontData = true; + Font font = new Font(tFile, fontFormat, true, tracker); + return font; + } finally { + if (!copiedFontData) { + if (tracker != null) { + tracker.subBytes(totalSize); + } + AccessController.doPrivileged( + new PrivilegedExceptionAction() { + public Void run() { + tFile.delete(); + return null; + } + } + ); + } + } + } catch (Throwable t) { + if (t instanceof FontFormatException) { + throw (FontFormatException)t; + } + if (t instanceof IOException) { + throw (IOException)t; + } + Throwable cause = t.getCause(); + if (cause instanceof FontFormatException) { + throw (FontFormatException)cause; + } + throw new IOException("Problem reading font data."); } } @@ -913,6 +979,9 @@ public class Font implements java.io.Serializable */ public static Font createFont(int fontFormat, File fontFile) throws java.awt.FontFormatException, java.io.IOException { + + fontFile = new File(fontFile.getPath()); + if (fontFormat != Font.TRUETYPE_FONT && fontFormat != Font.TYPE1_FONT) { throw new IllegalArgumentException ("font format not recognized"); @@ -926,7 +995,7 @@ public class Font implements java.io.Serializable if (!fontFile.canRead()) { throw new IOException("Can't read " + fontFile); } - return new Font(fontFile, fontFormat, false); + return new Font(fontFile, fontFormat, false, null); } /** diff --git a/jdk/src/share/classes/sun/font/CreatedFontTracker.java b/jdk/src/share/classes/sun/font/CreatedFontTracker.java new file mode 100644 index 00000000000..741337d5b19 --- /dev/null +++ b/jdk/src/share/classes/sun/font/CreatedFontTracker.java @@ -0,0 +1,54 @@ +/* + * Copyright 2008 Sun Microsystems, Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Sun designates this + * particular file as subject to the "Classpath" exception as provided + * by Sun in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, + * CA 95054 USA or visit www.sun.com if you need additional information or + * have any questions. + */ + +package sun.font; + +public class CreatedFontTracker { + + public static final int MAX_FILE_SIZE = 32 * 1024 * 1024; + public static final int MAX_TOTAL_BYTES = 10 * MAX_FILE_SIZE; + + static int numBytes; + static CreatedFontTracker tracker; + + public static synchronized CreatedFontTracker getTracker() { + if (tracker == null) { + tracker = new CreatedFontTracker(); + } + return tracker; + } + + public synchronized int getNumBytes() { + return numBytes; + } + + public synchronized void addBytes(int sz) { + numBytes += sz; + } + + public synchronized void subBytes(int sz) { + numBytes -= sz; + } +} diff --git a/jdk/src/share/classes/sun/font/FileFont.java b/jdk/src/share/classes/sun/font/FileFont.java index b6a2099d2a4..5aad11b2acd 100644 --- a/jdk/src/share/classes/sun/font/FileFont.java +++ b/jdk/src/share/classes/sun/font/FileFont.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2006 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2003-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -125,9 +125,9 @@ public abstract class FileFont extends PhysicalFont { return true; } - void setFileToRemove(File file) { + void setFileToRemove(File file, CreatedFontTracker tracker) { Disposer.addObjectRecord(this, - new CreatedFontFileDisposerRecord(file)); + new CreatedFontFileDisposerRecord(file, tracker)); } /* This is called when a font scaler is determined to @@ -246,12 +246,16 @@ public abstract class FileFont extends PhysicalFont { return getScaler().getUnitsPerEm(); } - private static class CreatedFontFileDisposerRecord implements DisposerRecord { + private static class CreatedFontFileDisposerRecord + implements DisposerRecord { File fontFile = null; + CreatedFontTracker tracker; - private CreatedFontFileDisposerRecord(File file) { + private CreatedFontFileDisposerRecord(File file, + CreatedFontTracker tracker) { fontFile = file; + this.tracker = tracker; } public void dispose() { @@ -260,6 +264,9 @@ public abstract class FileFont extends PhysicalFont { public Object run() { if (fontFile != null) { try { + if (tracker != null) { + tracker.subBytes((int)fontFile.length()); + } /* REMIND: is it possible that the file is * still open? It will be closed when the * font2D is disposed but could this code diff --git a/jdk/src/share/classes/sun/font/FontManager.java b/jdk/src/share/classes/sun/font/FontManager.java index 09f181f12c2..29d14047fbd 100644 --- a/jdk/src/share/classes/sun/font/FontManager.java +++ b/jdk/src/share/classes/sun/font/FontManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2007 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2003-2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -2347,12 +2347,14 @@ public final class FontManager { static Vector tmpFontFiles = null; public static Font2D createFont2D(File fontFile, int fontFormat, - boolean isCopy) + boolean isCopy, + CreatedFontTracker tracker) throws FontFormatException { String fontFilePath = fontFile.getPath(); FileFont font2D = null; final File fFile = fontFile; + final CreatedFontTracker _tracker = tracker; try { switch (fontFormat) { case Font.TRUETYPE_FONT: @@ -2369,6 +2371,9 @@ public final class FontManager { java.security.AccessController.doPrivileged( new java.security.PrivilegedAction() { public Object run() { + if (_tracker != null) { + _tracker.subBytes((int)fFile.length()); + } fFile.delete(); return null; } @@ -2377,7 +2382,7 @@ public final class FontManager { throw(e); } if (isCopy) { - font2D.setFileToRemove(fontFile); + font2D.setFileToRemove(fontFile, tracker); synchronized (FontManager.class) { if (tmpFontFiles == null) { diff --git a/jdk/test/java/awt/FontClass/CreateFont/A.ttf b/jdk/test/java/awt/FontClass/CreateFont/A.ttf new file mode 100644 index 0000000000000000000000000000000000000000..f80f5c3d569c810493a9fbaf7141a63831733764 GIT binary patch literal 2348 zcmbVO&2JM&6n``O>5tg!+OZ8Ju*6B67Kmdzu@eZ5A>f26!6qam0n%iX7&}rN8^;NW z+5=T7qF*Yg66&Fss;9P53qs;ULRDG`Rh4>aFTHT!*gt^wfH3Zx-EDxNwrbyc=KX$e z-p>4HcXq}=0Kf@tz`=Uzcqo$kYWxcTS;gpybnn3Mz}LUrL%xDMHj~$i@CUsNAl^lO ze`dL?-W<7o8h~3zUU#OL&A%P}K81V@xhFfndS=h!XE^{~0igZ4+5F0tmS7P;`UKlA zS4NH^stYxw-~>G5-UMwH8a6;yvys2kN_zyd8_5G-GYpUZ^(_ z0R<*aOu+%mCgz|Y)=kV~4(pYf07p%0~3&8)Wj4dD43Xo zM)=gkJmy?5u>b)gn^**yoHeo0KL>=jVBbu?ferlzHuM|V&~IQvzkv%oE)mw{@ z)ALk(@U0LYSt!qy=QFb~qG!!v<~+N9D3e{9*Ge#iBnwLz)S&ct$%EQVsgTdi&S@_w zQ#bAz#2#i)LILu+z&YgqDi1R@3Hzt9Iz!=bsH+S9?@*l(f-qho=z^{d;hO|Ng23+O z!|FAz>2N^H`2)GBLwj(Y$5HEU*a}V(C2+E2T)=~vut<_95`xR_!`r0#&f(mljo%H9 zCSlhNfyeqfr5L}?^H>9g9?ZYVskoLTIFV3^EGH~f;yD7Mz;RUId5}b&P{OhGoLmmY z{m|1L>Q*Z3M?&2dB$aLmw~-^cJGe0ABRTC_VrZ<>6~6;=b-i;u>e}gQin;Vq^)!L7tF(X_-gOujyG`qats(Gw%X$A^v$BoDRGDU4xg z3}YBd4@A5ma{C*&qpxtCazD3!8&x{^-RXYbB5xOJ8u@)5-oKC308vcEJ)w$%moB$L zLX{B8#!9KQzG(1bsMVaF0@}0F)7bQy#6Ux^xuZQE5G8~R#e7}{#afRy8i~g`+5-&| zHr9F=^F<^F(eq+auX{W3d41k!EE);Mg3Up1tw##_g1)#Em(T^##c7J~sI}H{({0KA z-j_R^t}QB0M5S)0XMQ}L=?b^o=;+`MDsekCJE9)TT*aOmpXm`_JF_>RFSMVX1LPrFW4L%88ih_kJsHFRWl@0Y z8{jd}rdmW!FpKSzO?n$HLEYBO9kGt>fEZcBeHhOvv|h7CCwfIN$1roer%js>>j@8w zP0{~Oc|0L_JY(@7LeFNP!og}4xe5n#&*mk9h?v6+K_5cFTDAxR2!g;_Mdn4$CSjT= z@pjYK$P!4B%sUJ(I4v^Bc$Z>j-YU4P7O+^X;!8H>ZKB&|1*_F2dhE>GrP>-B*labD z*YrMz9qe|8>~}Kn)K49985L1_m%fLJJ~BMH2NAXb;MvY9!=8Img9mztAi-ve-!qy~ z<2(ℜyHe$X3A=a4KE;YvWT*tD?oL)h8oIhlkqO(__Z&$MRa%9hh`>|BkPR{a1h+ zzaGW4_0-c8?qKZQQFBDRhf}MasAB@-4w~9ff>bwAkBE3W81tZb0w=(VZDVi?+sL1^ CwQRot literal 0 HcmV?d00001 diff --git a/jdk/test/java/awt/FontClass/CreateFont/BigFont.java b/jdk/test/java/awt/FontClass/CreateFont/BigFont.java new file mode 100644 index 00000000000..c14802485bc --- /dev/null +++ b/jdk/test/java/awt/FontClass/CreateFont/BigFont.java @@ -0,0 +1,139 @@ +/* + * Copyright 2008 Sun Microsystems, Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, + * CA 95054 USA or visit www.sun.com if you need additional information or + * have any questions. + */ + +import java.applet.*; +import java.awt.*; +import java.io.*; +import java.net.*; + +public class BigFont extends Applet { + + static private class SizedInputStream extends InputStream { + + int size; + int cnt = 0; + + SizedInputStream(int size) { + this.size = size; + } + + public int read() { + if (cnt < size) { + cnt++; + return 0; + } else { + return -1; + } + } + + public int getCurrentSize() { + return cnt; + } + } + + String id; + String fileName; + + public void init() { + id = getParameter("number"); + fileName = getParameter("font"); + + System.out.println("Applet " + id + " "+ + Thread.currentThread().getThreadGroup()); + // Larger than size for a single font. + int fontSize = 64 * 1000 * 1000; + SizedInputStream sis = new SizedInputStream(fontSize); + try { + Font font = Font.createFont(Font.TRUETYPE_FONT, sis); + } catch (Throwable t) { + if (t instanceof FontFormatException || + fontSize <= sis.getCurrentSize()) + { + System.out.println(sis.getCurrentSize()); + System.out.println(t); + throw new RuntimeException("Allowed file to be too large."); + } + } + // The following part of the test was verified manually but + // is impractical to enable because it requires a fairly large + // valid font to be part of the test, and we can't easily include + // that, nor dependably reference one from the applet environment. + /* + if (fileName == null) { + return; + } + int size = getFileSize(fileName); + if (size == 0) { + return; + } + int fontCnt = 1000 * 1000 * 1000 / size; + loadMany(size, fontCnt, fileName); + System.gc(); System.gc(); + fontCnt = fontCnt / 2; + System.out.println("Applet " + id + " load more."); + loadMany(size, fontCnt, fileName); + */ + System.out.println("Applet " + id + " finished."); + } + + int getFileSize(String fileName) { + try { + URL url = new URL(getCodeBase(), fileName); + InputStream inStream = url.openStream(); + BufferedInputStream fontStream = new BufferedInputStream(inStream); + int size = 0; + while (fontStream.read() != -1) { + size++; + } + fontStream.close(); + return size; + } catch (IOException e) { + return 0; + } + + } + void loadMany(int oneFont, int fontCnt, String fileName) { + System.out.println("fontcnt= " + fontCnt); + Font[] fonts = new Font[fontCnt]; + int totalSize = 0; + boolean gotException = false; + for (int i=0; i + + + + Test Font Creation Limits + + +
+ + + + + + + + +
+ + + diff --git a/jdk/test/java/awt/FontClass/CreateFont/fileaccess/FontFile.java b/jdk/test/java/awt/FontClass/CreateFont/fileaccess/FontFile.java new file mode 100644 index 00000000000..44419748903 --- /dev/null +++ b/jdk/test/java/awt/FontClass/CreateFont/fileaccess/FontFile.java @@ -0,0 +1,83 @@ +/* + * Copyright 2008 Sun Microsystems, Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, + * CA 95054 USA or visit www.sun.com if you need additional information or + * have any questions. + */ + +/* + * @test + * @bug 6652929 + * @summary verify handling of File.getPath() + */ + +import java.awt.*; +import java.io.*; + +public class FontFile { + public static void main(String[] args) throws Exception { + String sep = System.getProperty("file.separator"); + String fname = ".." + sep + "A.ttf"; + String dir = System.getProperty("test.src"); + if (dir != null) { + fname = dir + sep + fname; + } + final String name = fname; + System.out.println("Will try to access " + name); + if (!(new File(name)).canRead()) { + System.out.println("File not available : can't run test"); + return; + } + System.out.println("File is available. Verify no access under SM"); + + System.setSecurityManager(new SecurityManager()); + + + // Check cannot read file. + try { + new FileInputStream(name); + throw new Error("Something wrong with test environment"); + } catch (SecurityException exc) { + // Good. + } + + try { + Font font = Font.createFont(Font.TRUETYPE_FONT, + new File("nosuchfile") { + private boolean read; + @Override public String getPath() { + if (read) { + return name; + } else { + read = true; + return "somefile"; + } + } + @Override public boolean canRead() { + return true; + } + } + ); + System.err.println(font.getFontName()); + throw new RuntimeException("No expected exception"); + } catch (IOException e) { + System.err.println("Test passed."); + } + } +} From dcbd65a1c35d8e693d35275ae2a92e7ce891d32b Mon Sep 17 00:00:00 2001 From: Kumar Srinivasan Date: Wed, 18 Feb 2009 14:14:03 -0800 Subject: [PATCH 12/23] 6792554: Java JAR Pack200 header checks are insufficent Added several checks to ensure that the values read from the headers are consistent Reviewed-by: jrose --- .../com/sun/java/util/jar/pack/bands.cpp | 15 +- .../com/sun/java/util/jar/pack/coding.cpp | 3 +- .../com/sun/java/util/jar/pack/defines.h | 4 +- .../com/sun/java/util/jar/pack/unpack.cpp | 85 +++- .../tools/pack200/MemoryAllocatorTest.java | 369 ------------------ 5 files changed, 97 insertions(+), 379 deletions(-) delete mode 100644 jdk/test/tools/pack200/MemoryAllocatorTest.java diff --git a/jdk/src/share/native/com/sun/java/util/jar/pack/bands.cpp b/jdk/src/share/native/com/sun/java/util/jar/pack/bands.cpp index d900404dec4..e96d61c6b06 100644 --- a/jdk/src/share/native/com/sun/java/util/jar/pack/bands.cpp +++ b/jdk/src/share/native/com/sun/java/util/jar/pack/bands.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2002-2005 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2002-2009 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -94,6 +94,7 @@ void band::readData(int expectedLength) { assert(!valc->isMalloc); } xvs.init(u->rp, u->rplimit, valc); + CHECK; int X = xvs.getInt(); if (valc->S() != 0) { assert(valc->min <= -256); @@ -117,6 +118,7 @@ void band::readData(int expectedLength) { byte XB_byte = (byte) XB; byte* XB_ptr = &XB_byte; cm.init(u->rp, u->rplimit, XB_ptr, 0, defc, length, null); + CHECK; } else { NOT_PRODUCT(byte* meta_rp0 = u->meta_rp); assert(u->meta_rp != null); @@ -215,8 +217,19 @@ int band::getIntTotal() { if (length == 0) return 0; if (total_memo > 0) return total_memo-1; int total = getInt(); + // overflow checks require that none of the addends are <0, + // and that the partial sums never overflow (wrap negative) + if (total < 0) { + abort("overflow detected"); + return 0; + } for (int k = length-1; k > 0; k--) { + int prev_total = total; total += vs[0].getInt(); + if (total < prev_total) { + abort("overflow detected"); + return 0; + } } rewind(); total_memo = total+1; diff --git a/jdk/src/share/native/com/sun/java/util/jar/pack/coding.cpp b/jdk/src/share/native/com/sun/java/util/jar/pack/coding.cpp index d3ec4ae0e22..713ca999c98 100644 --- a/jdk/src/share/native/com/sun/java/util/jar/pack/coding.cpp +++ b/jdk/src/share/native/com/sun/java/util/jar/pack/coding.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2002-2005 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2002-2009 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -809,6 +809,7 @@ void coding_method::init(byte* &band_rp, byte* band_limit, } band_rp = vs.rp; } + CHECK; // Get an accurate upper limit now. vs0.rplimit = band_rp; diff --git a/jdk/src/share/native/com/sun/java/util/jar/pack/defines.h b/jdk/src/share/native/com/sun/java/util/jar/pack/defines.h index f780f247b2a..fa280ffb054 100644 --- a/jdk/src/share/native/com/sun/java/util/jar/pack/defines.h +++ b/jdk/src/share/native/com/sun/java/util/jar/pack/defines.h @@ -1,5 +1,5 @@ /* - * Copyright 2001-2008 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2001-2009 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -155,6 +155,8 @@ enum { false, true }; #define CHECK_NULL_(y,p) _CHECK_DO((p)==null, return y) #define CHECK_NULL_0(p) _CHECK_DO((p)==null, return 0) +#define CHECK_COUNT(t) if (t < 0){abort("bad value count");} CHECK + #define STR_TRUE "true" #define STR_FALSE "false" diff --git a/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.cpp b/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.cpp index 08fbbc2aa4a..e3a6945f30b 100644 --- a/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.cpp +++ b/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2008 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2001-2009 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -504,15 +504,39 @@ void unpacker::read_file_header() { enum { MAGIC_BYTES = 4, AH_LENGTH_0 = 3, //minver, majver, options are outside of archive_size + AH_LENGTH_0_MAX = AH_LENGTH_0 + 1, // options might have 2 bytes AH_LENGTH = 26, //maximum archive header length (w/ all fields) // Length contributions from optional header fields: AH_FILE_HEADER_LEN = 5, // sizehi/lo/next/modtime/files + AH_ARCHIVE_SIZE_LEN = 2, // sizehi/lo only; part of AH_FILE_HEADER_LEN AH_CP_NUMBER_LEN = 4, // int/float/long/double AH_SPECIAL_FORMAT_LEN = 2, // layouts/band-headers AH_LENGTH_MIN = AH_LENGTH -(AH_FILE_HEADER_LEN+AH_SPECIAL_FORMAT_LEN+AH_CP_NUMBER_LEN), + ARCHIVE_SIZE_MIN = AH_LENGTH_MIN - (AH_LENGTH_0 + AH_ARCHIVE_SIZE_LEN), FIRST_READ = MAGIC_BYTES + AH_LENGTH_MIN }; + + assert(AH_LENGTH_MIN == 15); // # of UNSIGNED5 fields required after archive_magic + assert(ARCHIVE_SIZE_MIN == 10); // # of UNSIGNED5 fields required after archive_size + // An absolute minimum null archive is magic[4], {minver,majver,options}[3], + // archive_size[0], cp_counts[8], class_counts[4], for a total of 19 bytes. + // (Note that archive_size is optional; it may be 0..10 bytes in length.) + // The first read must capture everything up through the options field. + // This happens to work even if {minver,majver,options} is a pathological + // 15 bytes long. Legal pack files limit those three fields to 1+1+2 bytes. + assert(FIRST_READ >= MAGIC_BYTES + AH_LENGTH_0 * B_MAX); + + // Up through archive_size, the largest possible archive header is + // magic[4], {minver,majver,options}[4], archive_size[10]. + // (Note only the low 12 bits of options are allowed to be non-zero.) + // In order to parse archive_size, we need at least this many bytes + // in the first read. Of course, if archive_size_hi is more than + // a byte, we probably will fail to allocate the buffer, since it + // will be many gigabytes long. This is a practical, not an + // architectural limit to Pack200 archive sizes. + assert(FIRST_READ >= MAGIC_BYTES + AH_LENGTH_0_MAX + 2*B_MAX); + bool foreign_buf = (read_input_fn == null); byte initbuf[FIRST_READ + C_SLOP + 200]; // 200 is for JAR I/O if (foreign_buf) { @@ -528,7 +552,7 @@ void unpacker::read_file_header() { // There is no way to tell the caller that we used only part of them. // Therefore, the caller must use only a bare minimum of read-ahead. if (inbytes.len > FIRST_READ) { - abort("too much pushback"); + abort("too much read-ahead"); return; } input.set(initbuf, sizeof(initbuf)); @@ -538,7 +562,7 @@ void unpacker::read_file_header() { rplimit += inbytes.len; bytes_read += inbytes.len; } - // Read only 19 bytes, which is certain to contain #archive_size fields, + // Read only 19 bytes, which is certain to contain #archive_options fields, // but is certain not to overflow past the archive_header. input.b.len = FIRST_READ; if (!ensure_input(FIRST_READ)) @@ -610,9 +634,9 @@ void unpacker::read_file_header() { #undef ORBIT if ((archive_options & ~OPTION_LIMIT) != 0) { fprintf(errstrm, "Warning: Illegal archive options 0x%x\n", - archive_options); - // Do not abort. If the format really changes, version numbers will bump. - //abort("illegal archive options"); + archive_options); + abort("illegal archive options"); + return; } if ((archive_options & AO_HAVE_FILE_HEADERS) != 0) { @@ -643,8 +667,17 @@ void unpacker::read_file_header() { return; } } else if (archive_size != 0) { + if (archive_size < ARCHIVE_SIZE_MIN) { + abort("impossible archive size"); // bad input data + return; + } + if (archive_size < header_size_1) { + abort("too much read-ahead"); // somehow we pre-fetched too much? + return; + } input.set(U_NEW(byte, add_size(header_size_0, archive_size, C_SLOP)), (size_t) header_size_0 + archive_size); + CHECK; assert(input.limit()[0] == 0); // Move all the bytes we read initially into the real buffer. input.b.copyFrom(initbuf, header_size); @@ -659,6 +692,7 @@ void unpacker::read_file_header() { rp = rplimit = input.base(); // Set up input buffer as if we already read the header: input.b.copyFrom(initbuf, header_size); + CHECK; rplimit += header_size; while (ensure_input(input.limit() - rp)) { size_t dataSoFar = input_remaining(); @@ -694,8 +728,10 @@ void unpacker::read_file_header() { if ((archive_options & AO_HAVE_FILE_HEADERS) != 0) { archive_next_count = hdr.getInt(); + CHECK_COUNT(archive_next_count); archive_modtime = hdr.getInt(); file_count = hdr.getInt(); + CHECK_COUNT(file_count); hdrVals += 3; } else { hdrValsSkipped += 3; @@ -703,7 +739,9 @@ void unpacker::read_file_header() { if ((archive_options & AO_HAVE_SPECIAL_FORMATS) != 0) { band_headers_size = hdr.getInt(); + CHECK_COUNT(band_headers_size); attr_definition_count = hdr.getInt(); + CHECK_COUNT(attr_definition_count); hdrVals += 2; } else { hdrValsSkipped += 2; @@ -723,13 +761,16 @@ void unpacker::read_file_header() { } } cp_counts[k] = hdr.getInt(); + CHECK_COUNT(cp_counts[k]); hdrVals += 1; } ic_count = hdr.getInt(); + CHECK_COUNT(ic_count); default_class_minver = hdr.getInt(); default_class_majver = hdr.getInt(); class_count = hdr.getInt(); + CHECK_COUNT(class_count); hdrVals += 4; // done with archive_header @@ -783,7 +824,6 @@ void unpacker::read_file_header() { bytes::of(band_headers.limit(), C_SLOP).clear(_meta_error); } - void unpacker::finish() { if (verbose >= 1) { fprintf(errstrm, @@ -2089,6 +2129,7 @@ void unpacker::read_classes() { field_descr.readData(field_count); read_attrs(ATTR_CONTEXT_FIELD, field_count); + CHECK; method_descr.readData(method_count); read_attrs(ATTR_CONTEXT_METHOD, method_count); @@ -2096,6 +2137,7 @@ void unpacker::read_classes() { CHECK; read_attrs(ATTR_CONTEXT_CLASS, class_count); + CHECK; read_code_headers(); @@ -2122,10 +2164,12 @@ void unpacker::read_attrs(int attrc, int obj_count) { assert(endsWith(xxx_flags_hi.name, "_flags_hi")); if (haveLongFlags) xxx_flags_hi.readData(obj_count); + CHECK; band& xxx_flags_lo = ad.xxx_flags_lo(); assert(endsWith(xxx_flags_lo.name, "_flags_lo")); xxx_flags_lo.readData(obj_count); + CHECK; // pre-scan flags, counting occurrences of each index bit julong indexMask = ad.flagIndexMask(); // which flag bits are index bits? @@ -2148,11 +2192,13 @@ void unpacker::read_attrs(int attrc, int obj_count) { assert(endsWith(xxx_attr_count.name, "_attr_count")); // There is one count element for each 1<<16 bit set in flags: xxx_attr_count.readData(ad.predefCount(X_ATTR_OVERFLOW)); + CHECK; band& xxx_attr_indexes = ad.xxx_attr_indexes(); assert(endsWith(xxx_attr_indexes.name, "_attr_indexes")); int overflowIndexCount = xxx_attr_count.getIntTotal(); xxx_attr_indexes.readData(overflowIndexCount); + CHECK; // pre-scan attr indexes, counting occurrences of each value for (i = 0; i < overflowIndexCount; i++) { idx = xxx_attr_indexes.getInt(); @@ -2183,6 +2229,7 @@ void unpacker::read_attrs(int attrc, int obj_count) { } } ad.xxx_attr_calls().readData(backwardCounts); + CHECK; // Read built-in bands. // Mostly, these are hand-coded equivalents to readBandData(). @@ -2191,42 +2238,53 @@ void unpacker::read_attrs(int attrc, int obj_count) { count = ad.predefCount(CLASS_ATTR_SourceFile); class_SourceFile_RUN.readData(count); + CHECK; count = ad.predefCount(CLASS_ATTR_EnclosingMethod); class_EnclosingMethod_RC.readData(count); class_EnclosingMethod_RDN.readData(count); + CHECK; count = ad.predefCount(X_ATTR_Signature); class_Signature_RS.readData(count); + CHECK; ad.readBandData(X_ATTR_RuntimeVisibleAnnotations); ad.readBandData(X_ATTR_RuntimeInvisibleAnnotations); count = ad.predefCount(CLASS_ATTR_InnerClasses); class_InnerClasses_N.readData(count); + CHECK; + count = class_InnerClasses_N.getIntTotal(); class_InnerClasses_RC.readData(count); class_InnerClasses_F.readData(count); + CHECK; // Drop remaining columns wherever flags are zero: count -= class_InnerClasses_F.getIntCount(0); class_InnerClasses_outer_RCN.readData(count); class_InnerClasses_name_RUN.readData(count); + CHECK; count = ad.predefCount(CLASS_ATTR_ClassFile_version); class_ClassFile_version_minor_H.readData(count); class_ClassFile_version_major_H.readData(count); + CHECK; break; case ATTR_CONTEXT_FIELD: count = ad.predefCount(FIELD_ATTR_ConstantValue); field_ConstantValue_KQ.readData(count); + CHECK; count = ad.predefCount(X_ATTR_Signature); field_Signature_RS.readData(count); + CHECK; ad.readBandData(X_ATTR_RuntimeVisibleAnnotations); ad.readBandData(X_ATTR_RuntimeInvisibleAnnotations); + CHECK; break; case ATTR_CONTEXT_METHOD: @@ -2238,15 +2296,18 @@ void unpacker::read_attrs(int attrc, int obj_count) { method_Exceptions_N.readData(count); count = method_Exceptions_N.getIntTotal(); method_Exceptions_RC.readData(count); + CHECK; count = ad.predefCount(X_ATTR_Signature); method_Signature_RS.readData(count); + CHECK; ad.readBandData(X_ATTR_RuntimeVisibleAnnotations); ad.readBandData(X_ATTR_RuntimeInvisibleAnnotations); ad.readBandData(METHOD_ATTR_RuntimeVisibleParameterAnnotations); ad.readBandData(METHOD_ATTR_RuntimeInvisibleParameterAnnotations); ad.readBandData(METHOD_ATTR_AnnotationDefault); + CHECK; break; case ATTR_CONTEXT_CODE: @@ -2258,8 +2319,10 @@ void unpacker::read_attrs(int attrc, int obj_count) { return; } code_StackMapTable_N.readData(count); + CHECK; count = code_StackMapTable_N.getIntTotal(); code_StackMapTable_frame_T.readData(count); + CHECK; // the rest of it depends in a complicated way on frame tags { int fat_frame_count = 0; @@ -2293,18 +2356,23 @@ void unpacker::read_attrs(int attrc, int obj_count) { // deal completely with fat frames: offset_count += fat_frame_count; code_StackMapTable_local_N.readData(fat_frame_count); + CHECK; type_count += code_StackMapTable_local_N.getIntTotal(); code_StackMapTable_stack_N.readData(fat_frame_count); type_count += code_StackMapTable_stack_N.getIntTotal(); + CHECK; // read the rest: code_StackMapTable_offset.readData(offset_count); code_StackMapTable_T.readData(type_count); + CHECK; // (7) [RCH] count = code_StackMapTable_T.getIntCount(7); code_StackMapTable_RC.readData(count); + CHECK; // (8) [PH] count = code_StackMapTable_T.getIntCount(8); code_StackMapTable_P.readData(count); + CHECK; } count = ad.predefCount(CODE_ATTR_LineNumberTable); @@ -2626,6 +2694,7 @@ void unpacker::read_code_headers() { code_max_na_locals.readData(); code_handler_count.readData(); totalHandlerCount += code_handler_count.getIntTotal(); + CHECK; // Read handler specifications. // Cf. PackageReader.readCodeHandlers. @@ -2633,8 +2702,10 @@ void unpacker::read_code_headers() { code_handler_end_PO.readData(totalHandlerCount); code_handler_catch_PO.readData(totalHandlerCount); code_handler_class_RCN.readData(totalHandlerCount); + CHECK; read_attrs(ATTR_CONTEXT_CODE, totalFlagsCount); + CHECK; } static inline bool is_in_range(uint n, uint min, uint max) { diff --git a/jdk/test/tools/pack200/MemoryAllocatorTest.java b/jdk/test/tools/pack200/MemoryAllocatorTest.java deleted file mode 100644 index 6015a4e470d..00000000000 --- a/jdk/test/tools/pack200/MemoryAllocatorTest.java +++ /dev/null @@ -1,369 +0,0 @@ -/* - * Copyright 2008 Sun Microsystems, Inc. All Rights Reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, - * CA 95054 USA or visit www.sun.com if you need additional information or - * have any questions. - */ - -/* - * @test - * @bug 6755943 - * @summary Checks any memory overruns in archive length. - * @run main/timeout=1200 MemoryAllocatorTest - */ -import java.io.BufferedReader; -import java.io.DataOutputStream; -import java.io.File; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.InputStreamReader; -import java.io.OutputStream; -import java.io.RandomAccessFile; -import java.nio.MappedByteBuffer; -import java.nio.channels.FileChannel; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; - -public class MemoryAllocatorTest { - - /* - * The smallest possible pack file with 1 empty resource - */ - static int[] magic = { - 0xCA, 0xFE, 0xD0, 0x0D - }; - static int[] version_info = { - 0x07, // minor - 0x96 // major - }; - static int[] option = { - 0x10 - }; - static int[] size_hi = { - 0x00 - }; - static int[] size_lo_ulong = { - 0xFF, 0xFC, 0xFC, 0xFC, 0xFC // ULONG_MAX 0xFFFFFFFF - }; - static int[] size_lo_correct = { - 0x17 - }; - static int[] data = { - 0x00, 0xEC, 0xDA, 0xDE, 0xF8, 0x45, 0x01, 0x02, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x01, 0x31, 0x01, 0x00 - }; - // End of pack file data - - static final String JAVA_HOME = System.getProperty("java.home"); - - static final boolean debug = Boolean.getBoolean("MemoryAllocatorTest.Debug"); - static final boolean WINDOWS = System.getProperty("os.name").startsWith("Windows"); - static final boolean LINUX = System.getProperty("os.name").startsWith("Linux"); - static final boolean SIXTYFOUR_BIT = System.getProperty("sun.arch.data.model", "32").equals("64"); - static final private int EXPECTED_EXIT_CODE = (WINDOWS) ? -1 : 255; - - static int testExitValue = 0; - - static byte[] bytes(int[] a) { - byte[] b = new byte[a.length]; - for (int i = 0; i < b.length; i++) { - b[i] = (byte) a[i]; - } - return b; - } - - static void createPackFile(boolean good, File packFile) throws IOException { - FileOutputStream fos = new FileOutputStream(packFile); - fos.write(bytes(magic)); - fos.write(bytes(version_info)); - fos.write(bytes(option)); - fos.write(bytes(size_hi)); - if (good) { - fos.write(bytes(size_lo_correct)); - } else { - fos.write(bytes(size_lo_ulong)); - } - fos.write(bytes(data)); - } - - /* - * This method modifies the LSB of the size_lo for various wicked - * values between MAXINT-0x3F and MAXINT. - */ - static int modifyPackFile(File packFile) throws IOException { - RandomAccessFile raf = new RandomAccessFile(packFile, "rws"); - long len = packFile.length(); - FileChannel fc = raf.getChannel(); - MappedByteBuffer bb = fc.map(FileChannel.MapMode.READ_WRITE, 0, len); - int pos = magic.length + version_info.length + option.length + - size_hi.length; - byte value = bb.get(pos); - value--; - bb.position(pos); - bb.put(value); - bb.force(); - fc.truncate(len); - fc.close(); - return value & 0xFF; - } - - static String getUnpack200Cmd() throws Exception { - File binDir = new File(JAVA_HOME, "bin"); - File unpack200File = WINDOWS - ? new File(binDir, "unpack200.exe") - : new File(binDir, "unpack200"); - - String cmd = unpack200File.getAbsolutePath(); - if (!unpack200File.canExecute()) { - throw new Exception("please check" + - cmd + " exists and is executable"); - } - return cmd; - } - - static TestResult runUnpacker(File packFile) throws Exception { - if (!packFile.exists()) { - throw new Exception("please check" + packFile + " exists"); - } - ArrayList alist = new ArrayList(); - ProcessBuilder pb = new ProcessBuilder(getUnpack200Cmd(), - packFile.getName(), "testout.jar"); - Map env = pb.environment(); - pb.directory(new File(".")); - int retval = 0; - try { - pb.redirectErrorStream(true); - Process p = pb.start(); - BufferedReader rd = new BufferedReader( - new InputStreamReader(p.getInputStream()), 8192); - String in = rd.readLine(); - while (in != null) { - alist.add(in); - System.out.println(in); - in = rd.readLine(); - } - retval = p.waitFor(); - p.destroy(); - } catch (Exception ex) { - ex.printStackTrace(); - throw new RuntimeException(ex.getMessage()); - } - return new TestResult("", retval, alist); - } - - /* - * The debug version builds of unpack200 call abort(3) which might set - * an unexpected return value, therefore this test is to determine - * if we are using a product or non-product build and check the - * return value appropriately. - */ - static boolean isNonProductVersion() throws Exception { - ArrayList alist = new ArrayList(); - ProcessBuilder pb = new ProcessBuilder(getUnpack200Cmd(), "--version"); - Map env = pb.environment(); - pb.directory(new File(".")); - int retval = 0; - try { - pb.redirectErrorStream(true); - Process p = pb.start(); - BufferedReader rd = new BufferedReader( - new InputStreamReader(p.getInputStream()), 8192); - String in = rd.readLine(); - while (in != null) { - alist.add(in); - System.out.println(in); - in = rd.readLine(); - } - retval = p.waitFor(); - p.destroy(); - } catch (Exception ex) { - ex.printStackTrace(); - throw new RuntimeException(ex.getMessage()); - } - for (String x : alist) { - if (x.contains("non-product")) { - return true; - } - } - return false; - } - - /** - * @param args the command line arguments - * @throws java.lang.Exception - */ - public static void main(String[] args) throws Exception { - - File packFile = new File("tiny.pack"); - boolean isNPVersion = isNonProductVersion(); - - // Create a good pack file and test if everything is ok - createPackFile(true, packFile); - TestResult tr = runUnpacker(packFile); - tr.setDescription("a good pack file"); - tr.checkPositive(); - tr.isOK(); - System.out.println(tr); - - /* - * jprt systems on windows and linux seem to have abundant memory - * therefore can take a very long time to run, and even if it does - * the error message is not accurate for us to discern if the test - * passess successfully. - */ - if (SIXTYFOUR_BIT && (LINUX || WINDOWS)) { - System.out.println("Warning: Windows/Linux 64bit tests passes vacuously"); - return; - } - - /* - * debug builds call abort, the exit code under these conditions - * are not really relevant. - */ - if (isNPVersion) { - System.out.println("Warning: non-product build: exit values not checked"); - } - - // create a bad pack file - createPackFile(false, packFile); - tr = runUnpacker(packFile); - tr.setDescription("a wicked pack file"); - tr.contains("Native allocation failed"); - if(!isNPVersion) { - tr.checkValue(EXPECTED_EXIT_CODE); - } - System.out.println(tr); - int value = modifyPackFile(packFile); - tr.setDescription("value=" + value); - - // continue creating bad pack files by modifying the specimen pack file. - while (value >= 0xc0) { - tr = runUnpacker(packFile); - tr.contains("Native allocation failed"); - if (!isNPVersion) { - tr.checkValue(EXPECTED_EXIT_CODE); - } - tr.setDescription("wicked value=0x" + - Integer.toHexString(value & 0xFF)); - System.out.println(tr); - value = modifyPackFile(packFile); - } - if (testExitValue != 0) { - throw new Exception("Pack200 archive length tests(" + - testExitValue + ") failed "); - } else { - System.out.println("All tests pass"); - } - } - - /* - * A class to encapsulate the test results and stuff, with some ease - * of use methods to check the test results. - */ - static class TestResult { - - StringBuilder status; - int exitValue; - List testOutput; - String description; - - public TestResult(String str, int rv, List oList) { - status = new StringBuilder(str); - exitValue = rv; - testOutput = oList; - } - - void setDescription(String description) { - this.description = description; - } - - void checkValue(int value) { - if (exitValue != value) { - status = - status.append(" Error: test expected exit value " + - value + "got " + exitValue); - testExitValue++; - } - } - - void checkNegative() { - if (exitValue == 0) { - status = status.append( - " Error: test did not expect 0 exit value"); - - testExitValue++; - } - } - - void checkPositive() { - if (exitValue != 0) { - status = status.append( - " Error: test did not return 0 exit value"); - testExitValue++; - } - } - - boolean isOK() { - return exitValue == 0; - } - - boolean isZeroOutput() { - if (!testOutput.isEmpty()) { - status = status.append(" Error: No message from cmd please"); - testExitValue++; - return false; - } - return true; - } - - boolean isNotZeroOutput() { - if (testOutput.isEmpty()) { - status = status.append(" Error: Missing message"); - testExitValue++; - return false; - } - return true; - } - - public String toString() { - if (debug) { - for (String x : testOutput) { - status = status.append(x + "\n"); - } - } - if (description != null) { - status.insert(0, description); - } - return status.append("\nexitValue = " + exitValue).toString(); - } - - boolean contains(String str) { - for (String x : testOutput) { - if (x.contains(str)) { - return true; - } - } - status = status.append(" Error: string <" + str + "> not found "); - testExitValue++; - return false; - } - } -} From 22f94de7e6abdcf00ef9893416ccf5c0c3436ab8 Mon Sep 17 00:00:00 2001 From: Andrew Brygin Date: Fri, 20 Feb 2009 13:48:32 +0300 Subject: [PATCH 13/23] 6804996: JWS PNG Decoding Integer Overflow [V-flrhat2ln8] Reviewed-by: prr --- .../sun/awt/splashscreen/splashscreen_gif.c | 4 ---- .../sun/awt/splashscreen/splashscreen_impl.h | 4 ++++ .../sun/awt/splashscreen/splashscreen_png.c | 23 +++++++++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/jdk/src/share/native/sun/awt/splashscreen/splashscreen_gif.c b/jdk/src/share/native/sun/awt/splashscreen/splashscreen_gif.c index 71bc3a3b39d..f1651ab1f49 100644 --- a/jdk/src/share/native/sun/awt/splashscreen/splashscreen_gif.c +++ b/jdk/src/share/native/sun/awt/splashscreen/splashscreen_gif.c @@ -53,10 +53,6 @@ static const char szNetscape20ext[11] = "NETSCAPE2.0"; // convert libungif samples to our ones #define MAKE_QUAD_GIF(c,a) MAKE_QUAD((c).Red, (c).Green, (c).Blue, (a)) -#define SAFE_TO_ALLOC(c, sz) \ - (((c) > 0) && ((sz) > 0) && \ - ((0xffffffffu / ((unsigned int)(c))) > (unsigned int)(sz))) - /* stdio FILE* and memory input functions for libungif */ int SplashStreamGifInputFunc(GifFileType * gif, GifByteType * buf, int n) diff --git a/jdk/src/share/native/sun/awt/splashscreen/splashscreen_impl.h b/jdk/src/share/native/sun/awt/splashscreen/splashscreen_impl.h index c6bad14c45a..6f4e03ef53e 100644 --- a/jdk/src/share/native/sun/awt/splashscreen/splashscreen_impl.h +++ b/jdk/src/share/native/sun/awt/splashscreen/splashscreen_impl.h @@ -155,6 +155,10 @@ int BitmapToYXBandedRectangles(ImageRect * pSrcRect, RECT_T * out); void SplashInitFrameShape(Splash * splash, int imageIndex); +#define SAFE_TO_ALLOC(c, sz) \ + (((c) > 0) && ((sz) > 0) && \ + ((0xffffffffu / ((unsigned int)(c))) > (unsigned int)(sz))) + #define dbgprintf printf #endif diff --git a/jdk/src/share/native/sun/awt/splashscreen/splashscreen_png.c b/jdk/src/share/native/sun/awt/splashscreen/splashscreen_png.c index f0926ec90dc..abc54dcb753 100644 --- a/jdk/src/share/native/sun/awt/splashscreen/splashscreen_png.c +++ b/jdk/src/share/native/sun/awt/splashscreen/splashscreen_png.c @@ -103,9 +103,17 @@ SplashDecodePng(Splash * splash, png_rw_ptr read_func, void *io_ptr) rowbytes = png_get_rowbytes(png_ptr, info_ptr); + if (!SAFE_TO_ALLOC(rowbytes, height)) { + goto done; + } + if ((image_data = (unsigned char *) malloc(rowbytes * height)) == NULL) { goto done; } + + if (!SAFE_TO_ALLOC(height, sizeof(png_bytep))) { + goto done; + } if ((row_pointers = (png_bytepp) malloc(height * sizeof(png_bytep))) == NULL) { goto done; @@ -121,13 +129,28 @@ SplashDecodePng(Splash * splash, png_rw_ptr read_func, void *io_ptr) splash->width = width; splash->height = height; + if (!SAFE_TO_ALLOC(splash->width, splash->imageFormat.depthBytes)) { + goto done; + } stride = splash->width * splash->imageFormat.depthBytes; + if (!SAFE_TO_ALLOC(splash->height, stride)) { + goto done; + } splash->frameCount = 1; splash->frames = (SplashImage *) malloc(sizeof(SplashImage) * splash->frameCount); + + if (splash->frames == NULL) { + goto done; + } + splash->loopCount = 1; splash->frames[0].bitmapBits = malloc(stride * splash->height); + if (splash->frames[0].bitmapBits == NULL) { + free(splash->frames); + goto done; + } splash->frames[0].delay = 0; /* FIXME: sort out the real format */ From de4c8e0eb58259a6a8f7261925a508ebc12b8cec Mon Sep 17 00:00:00 2001 From: Phil Race Date: Tue, 3 Mar 2009 16:10:37 -0800 Subject: [PATCH 14/23] 2163516: Font.createFont can be persuaded to leak temporary files Reviewed-by: igor --- .../share/classes/sun/font/FontManager.java | 2 +- .../share/classes/sun/font/TrueTypeFont.java | 13 ++++- jdk/src/share/classes/sun/font/Type1Font.java | 50 ++++++++++++++++++- .../awt/FontClass/CreateFont/DeleteFont.java | 14 ++++-- 4 files changed, 70 insertions(+), 9 deletions(-) diff --git a/jdk/src/share/classes/sun/font/FontManager.java b/jdk/src/share/classes/sun/font/FontManager.java index 29d14047fbd..aad8c5116c0 100644 --- a/jdk/src/share/classes/sun/font/FontManager.java +++ b/jdk/src/share/classes/sun/font/FontManager.java @@ -2361,7 +2361,7 @@ public final class FontManager { font2D = new TrueTypeFont(fontFilePath, null, 0, true); break; case Font.TYPE1_FONT: - font2D = new Type1Font(fontFilePath, null); + font2D = new Type1Font(fontFilePath, null, isCopy); break; default: throw new FontFormatException("Unrecognised Font Format"); diff --git a/jdk/src/share/classes/sun/font/TrueTypeFont.java b/jdk/src/share/classes/sun/font/TrueTypeFont.java index 8a8309efab4..e4784eec86b 100644 --- a/jdk/src/share/classes/sun/font/TrueTypeFont.java +++ b/jdk/src/share/classes/sun/font/TrueTypeFont.java @@ -174,8 +174,17 @@ public class TrueTypeFont extends FileFont { super(platname, nativeNames); useJavaRasterizer = javaRasterizer; fontRank = Font2D.TTF_RANK; - verify(); - init(fIndex); + try { + verify(); + init(fIndex); + } catch (Throwable t) { + close(); + if (t instanceof FontFormatException) { + throw (FontFormatException)t; + } else { + throw new FontFormatException("Unexpected runtime exception."); + } + } Disposer.addObjectRecord(this, disposerRecord); } diff --git a/jdk/src/share/classes/sun/font/Type1Font.java b/jdk/src/share/classes/sun/font/Type1Font.java index 90236ff64c1..e63303fbdbc 100644 --- a/jdk/src/share/classes/sun/font/Type1Font.java +++ b/jdk/src/share/classes/sun/font/Type1Font.java @@ -39,6 +39,7 @@ import java.nio.BufferUnderflowException; import java.nio.channels.ClosedChannelException; import java.nio.channels.FileChannel; import sun.java2d.Disposer; +import sun.java2d.DisposerRecord; import java.util.HashSet; import java.util.HashMap; import java.awt.Font; @@ -76,6 +77,27 @@ import java.awt.Font; */ public class Type1Font extends FileFont { + private static class T1DisposerRecord implements DisposerRecord { + String fileName = null; + + T1DisposerRecord(String name) { + fileName = name; + } + + public synchronized void dispose() { + java.security.AccessController.doPrivileged( + new java.security.PrivilegedAction() { + public Object run() { + + if (fileName != null) { + (new java.io.File(fileName)).delete(); + } + return null; + } + }); + } + } + WeakReference bufferRef = new WeakReference(null); private String psName = null; @@ -124,6 +146,17 @@ public class Type1Font extends FileFont { } + /** + * Constructs a Type1 Font. + * @param platname - Platform identifier of the font. Typically file name. + * @param nativeNames - Native names - typically XLFDs on Unix. + */ + public Type1Font(String platname, Object nativeNames) + throws FontFormatException { + + this(platname, nativeNames, false); + } + /** * - does basic verification of the file * - reads the names (full, family). @@ -131,12 +164,25 @@ public class Type1Font extends FileFont { * @throws FontFormatException - if the font can't be opened * or fails verification, or there's no usable cmap */ - public Type1Font(String platname, Object nativeNames) + public Type1Font(String platname, Object nativeNames, boolean createdCopy) throws FontFormatException { super(platname, nativeNames); fontRank = Font2D.TYPE1_RANK; checkedNatives = true; - verify(); + try { + verify(); + } catch (Throwable t) { + if (createdCopy) { + T1DisposerRecord ref = new T1DisposerRecord(platname); + Disposer.addObjectRecord(bufferRef, ref); + bufferRef = null; + } + if (t instanceof FontFormatException) { + throw (FontFormatException)t; + } else { + throw new FontFormatException("Unexpected runtime exception."); + } + } } private synchronized ByteBuffer getBuffer() throws FontFormatException { diff --git a/jdk/test/java/awt/FontClass/CreateFont/DeleteFont.java b/jdk/test/java/awt/FontClass/CreateFont/DeleteFont.java index eba9462f842..361b0b436ac 100644 --- a/jdk/test/java/awt/FontClass/CreateFont/DeleteFont.java +++ b/jdk/test/java/awt/FontClass/CreateFont/DeleteFont.java @@ -55,17 +55,23 @@ public class DeleteFont { if (!gotException) { throw new RuntimeException("No expected IOException"); } - badRead(-2); - badRead(8193); + badRead(-2, Font.TRUETYPE_FONT); + badRead(8193, Font.TRUETYPE_FONT); + + badRead(-2, Font.TYPE1_FONT); + badRead(8193, Font.TYPE1_FONT); + + // Make sure GC has a chance to clean up before we exit. + System.gc(); System.gc(); } - static void badRead(final int retval) { + static void badRead(final int retval, int fontType) { int num = 2; byte[] buff = new byte[16*8192]; // Multiple of 8192 is important. for (int ct=0; ct Date: Thu, 5 Mar 2009 19:36:51 +0300 Subject: [PATCH 15/23] 6804998: JRE GIF Decoding Heap Corruption [V-y6g5jlm8e1] Reviewed-by: prr --- .../share/classes/sun/awt/image/GifImageDecoder.java | 11 +++++++++-- jdk/src/share/native/sun/awt/image/gif/gifdecoder.c | 5 +++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/jdk/src/share/classes/sun/awt/image/GifImageDecoder.java b/jdk/src/share/classes/sun/awt/image/GifImageDecoder.java index aa87fb82ef7..b0679afc437 100644 --- a/jdk/src/share/classes/sun/awt/image/GifImageDecoder.java +++ b/jdk/src/share/classes/sun/awt/image/GifImageDecoder.java @@ -585,9 +585,16 @@ public class GifImageDecoder extends ImageDecoder { System.out.print("Reading a " + width + " by " + height + " " + (interlace ? "" : "non-") + "interlaced image..."); } - + int initCodeSize = ExtractByte(block, 9); + if (initCodeSize >= 12) { + if (verbose) { + System.out.println("Invalid initial code size: " + + initCodeSize); + } + return false; + } boolean ret = parseImage(x, y, width, height, - interlace, ExtractByte(block, 9), + interlace, initCodeSize, block, rasline, model); if (!ret) { diff --git a/jdk/src/share/native/sun/awt/image/gif/gifdecoder.c b/jdk/src/share/native/sun/awt/image/gif/gifdecoder.c index 1429931f8c6..893007f1366 100644 --- a/jdk/src/share/native/sun/awt/image/gif/gifdecoder.c +++ b/jdk/src/share/native/sun/awt/image/gif/gifdecoder.c @@ -191,6 +191,11 @@ Java_sun_awt_image_GifImageDecoder_parseImage(JNIEnv *env, int passht = passinc; int len; + /* We have verified the initial code size on the java layer. + * Here we just check bounds for particular indexes. */ + if (freeCode >= 4096 || maxCode >= 4096) { + return 0; + } if (blockh == 0 || raslineh == 0 || prefixh == 0 || suffixh == 0 || outCodeh == 0) From 6c11535cddb56103822a86c878bedf6932791008 Mon Sep 17 00:00:00 2001 From: Andrew Brygin Date: Fri, 6 Mar 2009 12:40:38 +0300 Subject: [PATCH 16/23] 6804997: JWS GIF Decoding Heap Corruption [V-r687oxuocp] Reviewed-by: prr --- jdk/src/share/native/sun/awt/giflib/dgif_lib.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/jdk/src/share/native/sun/awt/giflib/dgif_lib.c b/jdk/src/share/native/sun/awt/giflib/dgif_lib.c index be91d3e7811..f20372fe925 100644 --- a/jdk/src/share/native/sun/awt/giflib/dgif_lib.c +++ b/jdk/src/share/native/sun/awt/giflib/dgif_lib.c @@ -722,6 +722,10 @@ DGifSetupDecompress(GifFileType * GifFile) { GifFilePrivateType *Private = (GifFilePrivateType *)GifFile->Private; READ(GifFile, &CodeSize, 1); /* Read Code size from file. */ + if (CodeSize >= 12) { + /* Invalid initial code size: report failure */ + return GIF_ERROR; + } BitsPerPixel = CodeSize; Private->Buf[0] = 0; /* Input Buffer empty. */ @@ -964,10 +968,13 @@ DGifDecompressInput(GifFileType * GifFile, /* If code cannot fit into RunningBits bits, must raise its size. Note * however that codes above 4095 are used for special signaling. */ - if (++Private->RunningCode > Private->MaxCode1 && - Private->RunningBits < LZ_BITS) { - Private->MaxCode1 <<= 1; - Private->RunningBits++; + if (++Private->RunningCode > Private->MaxCode1) { + if (Private->RunningBits < LZ_BITS) { + Private->MaxCode1 <<= 1; + Private->RunningBits++; + } else { + Private->RunningCode = Private->MaxCode1; + } } return GIF_OK; } From 605e712ecd9b7e9088a7b79cd506ff1b088fdac2 Mon Sep 17 00:00:00 2001 From: Daniel Fuchs Date: Mon, 9 Mar 2009 21:49:56 +0100 Subject: [PATCH 17/23] 6656633: getNotificationInfo methods static mutable Reviewed-by: emcmanus, jfdenise --- .../classes/javax/management/monitor/CounterMonitor.java | 2 +- .../share/classes/javax/management/monitor/GaugeMonitor.java | 2 +- .../classes/javax/management/monitor/StringMonitor.java | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/jdk/src/share/classes/javax/management/monitor/CounterMonitor.java b/jdk/src/share/classes/javax/management/monitor/CounterMonitor.java index f2f9887b4cd..8105e6fa8aa 100644 --- a/jdk/src/share/classes/javax/management/monitor/CounterMonitor.java +++ b/jdk/src/share/classes/javax/management/monitor/CounterMonitor.java @@ -596,7 +596,7 @@ public class CounterMonitor extends Monitor implements CounterMonitorMBean { * types sent by the counter monitor. */ public MBeanNotificationInfo[] getNotificationInfo() { - return notifsInfo; + return notifsInfo.clone(); } /* diff --git a/jdk/src/share/classes/javax/management/monitor/GaugeMonitor.java b/jdk/src/share/classes/javax/management/monitor/GaugeMonitor.java index 5ce63fbe532..8fe8700464f 100644 --- a/jdk/src/share/classes/javax/management/monitor/GaugeMonitor.java +++ b/jdk/src/share/classes/javax/management/monitor/GaugeMonitor.java @@ -478,7 +478,7 @@ public class GaugeMonitor extends Monitor implements GaugeMonitorMBean { * types sent by the gauge monitor. */ public MBeanNotificationInfo[] getNotificationInfo() { - return notifsInfo; + return notifsInfo.clone(); } /* diff --git a/jdk/src/share/classes/javax/management/monitor/StringMonitor.java b/jdk/src/share/classes/javax/management/monitor/StringMonitor.java index 3c2bbc56410..9b4af52d41e 100644 --- a/jdk/src/share/classes/javax/management/monitor/StringMonitor.java +++ b/jdk/src/share/classes/javax/management/monitor/StringMonitor.java @@ -184,6 +184,7 @@ public class StringMonitor extends Monitor implements StringMonitorMBean { * @return The derived gauge of the specified object. * */ + @Override public synchronized String getDerivedGauge(ObjectName object) { return (String) super.getDerivedGauge(object); } @@ -199,6 +200,7 @@ public class StringMonitor extends Monitor implements StringMonitorMBean { * @return The derived gauge timestamp of the specified object. * */ + @Override public synchronized long getDerivedGaugeTimeStamp(ObjectName object) { return super.getDerivedGaugeTimeStamp(object); } @@ -341,8 +343,9 @@ public class StringMonitor extends Monitor implements StringMonitorMBean { * the Java class of the notification and the notification types sent by * the string monitor. */ + @Override public MBeanNotificationInfo[] getNotificationInfo() { - return notifsInfo; + return notifsInfo.clone(); } /* From 2f5bb727a1e15d0f1d2177080cda44dcee359c4c Mon Sep 17 00:00:00 2001 From: Daniel Fuchs Date: Mon, 9 Mar 2009 22:17:52 +0100 Subject: [PATCH 18/23] 6691246: Thread context class loader can be set using JMX remote ClientNotifForwarded Reviewed-by: emcmanus --- .../remote/internal/ClientNotifForwarder.java | 98 +++++++++++++++---- 1 file changed, 80 insertions(+), 18 deletions(-) diff --git a/jdk/src/share/classes/com/sun/jmx/remote/internal/ClientNotifForwarder.java b/jdk/src/share/classes/com/sun/jmx/remote/internal/ClientNotifForwarder.java index 7af1e5f47bc..0b876340e05 100644 --- a/jdk/src/share/classes/com/sun/jmx/remote/internal/ClientNotifForwarder.java +++ b/jdk/src/share/classes/com/sun/jmx/remote/internal/ClientNotifForwarder.java @@ -22,7 +22,6 @@ * CA 95054 USA or visit www.sun.com if you need additional information or * have any questions. */ - package com.sun.jmx.remote.internal; import java.io.IOException; @@ -34,6 +33,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.Executor; +import java.security.AccessControlContext; import java.security.AccessController; import java.security.PrivilegedAction; import javax.security.auth.Subject; @@ -54,6 +54,9 @@ import com.sun.jmx.remote.util.EnvHelp; public abstract class ClientNotifForwarder { + + private final AccessControlContext acc; + public ClientNotifForwarder(Map env) { this(null, env); } @@ -87,6 +90,8 @@ public abstract class ClientNotifForwarder { this.command = command; if (thread == null) { thread = new Thread() { + + @Override public void run() { while (true) { Runnable r; @@ -130,6 +135,7 @@ public abstract class ClientNotifForwarder { this.defaultClassLoader = defaultClassLoader; this.executor = ex; + this.acc = AccessController.getContext(); } /** @@ -390,28 +396,85 @@ public abstract class ClientNotifForwarder { setState(TERMINATED); } -// ------------------------------------------------- -// private classes -// ------------------------------------------------- + + // ------------------------------------------------- + // private classes + // ------------------------------------------------- // + private class NotifFetcher implements Runnable { + + private volatile boolean alreadyLogged = false; + + private void logOnce(String msg, SecurityException x) { + if (alreadyLogged) return; + // Log only once. + logger.config("setContextClassLoader",msg); + if (x != null) logger.fine("setContextClassLoader", x); + alreadyLogged = true; + } + + // Set new context class loader, returns previous one. + private final ClassLoader setContextClassLoader(final ClassLoader loader) { + final AccessControlContext ctxt = ClientNotifForwarder.this.acc; + // if ctxt is null, log a config message and throw a + // SecurityException. + if (ctxt == null) { + logOnce("AccessControlContext must not be null.",null); + throw new SecurityException("AccessControlContext must not be null"); + } + return AccessController.doPrivileged( + new PrivilegedAction() { + public ClassLoader run() { + try { + // get context class loader - may throw + // SecurityException - though unlikely. + final ClassLoader previous = + Thread.currentThread().getContextClassLoader(); + + // if nothing needs to be done, break here... + if (loader == previous) return previous; + + // reset context class loader - may throw + // SecurityException + Thread.currentThread().setContextClassLoader(loader); + return previous; + } catch (SecurityException x) { + logOnce("Permission to set ContextClassLoader missing. " + + "Notifications will not be dispatched. " + + "Please check your Java policy configuration: " + + x, x); + throw x; + } + } + }, ctxt); + } + public void run() { + final ClassLoader previous; + if (defaultClassLoader != null) { + previous = setContextClassLoader(defaultClassLoader); + } else { + previous = null; + } + try { + doRun(); + } finally { + if (defaultClassLoader != null) { + setContextClassLoader(previous); + } + } + } + + private void doRun() { synchronized (ClientNotifForwarder.this) { currentFetchThread = Thread.currentThread(); - if (state == STARTING) + if (state == STARTING) { setState(STARTED); + } } - if (defaultClassLoader != null) { - AccessController.doPrivileged(new PrivilegedAction() { - public Void run() { - Thread.currentThread(). - setContextClassLoader(defaultClassLoader); - return null; - } - }); - } NotificationResult nr = null; if (!shouldStop() && (nr = fetchNotifs()) != null) { @@ -444,8 +507,9 @@ public abstract class ClientNotifForwarder { // check if an mbean unregistration notif if (!listenerID.equals(mbeanRemovedNotifID)) { final ClientListenerInfo li = infoList.get(listenerID); - if (li != null) - listeners.put(listenerID,li); + if (li != null) { + listeners.put(listenerID, li); + } continue; } final Notification notif = tn.getNotification(); @@ -786,9 +850,7 @@ public abstract class ClientNotifForwarder { private long clientSequenceNumber = -1; private final int maxNotifications; private final long timeout; - private Integer mbeanRemovedNotifID = null; - private Thread currentFetchThread; // state From b047886b2f759d6bef024537432eecf9a4d70311 Mon Sep 17 00:00:00 2001 From: Daniel Fuchs Date: Mon, 9 Mar 2009 22:34:08 +0100 Subject: [PATCH 19/23] 6610888: Potential use of cleared of incorrect acc in JMX Monitor Reviewed-by: emcmanus --- .../javax/management/monitor/Monitor.java | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/jdk/src/share/classes/javax/management/monitor/Monitor.java b/jdk/src/share/classes/javax/management/monitor/Monitor.java index 1b057e1b4a6..75f3ae87771 100644 --- a/jdk/src/share/classes/javax/management/monitor/Monitor.java +++ b/jdk/src/share/classes/javax/management/monitor/Monitor.java @@ -32,6 +32,7 @@ import java.io.IOException; import java.security.AccessControlContext; import java.security.AccessController; import java.security.PrivilegedAction; +import java.security.ProtectionDomain; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ExecutorService; @@ -164,7 +165,10 @@ public abstract class Monitor /** * AccessControlContext of the Monitor.start() caller. */ - private AccessControlContext acc; + private static final AccessControlContext noPermissionsACC = + new AccessControlContext( + new ProtectionDomain[] {new ProtectionDomain(null, null)}); + private volatile AccessControlContext acc = noPermissionsACC; /** * Scheduler Service. @@ -749,7 +753,7 @@ public abstract class Monitor // Reset the AccessControlContext. // - acc = null; + acc = noPermissionsACC; // Reset the complex type attribute information // such that it is recalculated again. @@ -1518,10 +1522,12 @@ public abstract class Monitor public void run() { final ScheduledFuture sf; + final AccessControlContext ac; synchronized (Monitor.this) { sf = Monitor.this.schedulerFuture; + ac = Monitor.this.acc; } - AccessController.doPrivileged(new PrivilegedAction() { + PrivilegedAction action = new PrivilegedAction() { public Void run() { if (Monitor.this.isActive()) { final int an[] = alreadyNotifieds; @@ -1534,7 +1540,11 @@ public abstract class Monitor } return null; } - }, Monitor.this.acc); + }; + if (ac == null) { + throw new SecurityException("AccessControlContext cannot be null"); + } + AccessController.doPrivileged(action, ac); synchronized (Monitor.this) { if (Monitor.this.isActive() && Monitor.this.schedulerFuture == sf) { From fbcaea5fc158f43ff301239350232d25dc251120 Mon Sep 17 00:00:00 2001 From: Daniel Fuchs Date: Mon, 9 Mar 2009 22:49:21 +0100 Subject: [PATCH 20/23] 6610896: JMX Monitor handles thread groups incorrectly Reviewed-by: emcmanus --- .../javax/management/monitor/Monitor.java | 97 ++++++++++++++----- 1 file changed, 74 insertions(+), 23 deletions(-) diff --git a/jdk/src/share/classes/javax/management/monitor/Monitor.java b/jdk/src/share/classes/javax/management/monitor/Monitor.java index 1b057e1b4a6..df66414a1a0 100644 --- a/jdk/src/share/classes/javax/management/monitor/Monitor.java +++ b/jdk/src/share/classes/javax/management/monitor/Monitor.java @@ -33,8 +33,9 @@ import java.security.AccessControlContext; import java.security.AccessController; import java.security.PrivilegedAction; import java.util.List; +import java.util.Map; +import java.util.WeakHashMap; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; @@ -173,15 +174,21 @@ public abstract class Monitor Executors.newSingleThreadScheduledExecutor( new DaemonThreadFactory("Scheduler")); + /** + * Map containing the thread pool executor per thread group. + */ + private static final Map executors = + new WeakHashMap(); + + /** + * Lock for executors map. + */ + private static final Object executorsLock = new Object(); + /** * Maximum Pool Size */ private static final int maximumPoolSize; - - /** - * Executor Service. - */ - private static final ExecutorService executor; static { final String maximumPoolSizeSysProp = "jmx.x.monitor.maximum.pool.size"; final String maximumPoolSizeStr = AccessController.doPrivileged( @@ -211,21 +218,8 @@ public abstract class Monitor maximumPoolSize = maximumPoolSizeTmp; } } - executor = new ThreadPoolExecutor( - maximumPoolSize, - maximumPoolSize, - 60L, - TimeUnit.SECONDS, - new LinkedBlockingQueue(), - new DaemonThreadFactory("Executor")); - ((ThreadPoolExecutor)executor).allowCoreThreadTimeOut(true); } - /** - * Monitor task to be executed by the Executor Service. - */ - private final MonitorTask monitorTask = new MonitorTask(); - /** * Future associated to the current monitor task. */ @@ -234,7 +228,7 @@ public abstract class Monitor /** * Scheduler task to be executed by the Scheduler Service. */ - private final SchedulerTask schedulerTask = new SchedulerTask(monitorTask); + private final SchedulerTask schedulerTask = new SchedulerTask(); /** * ScheduledFuture associated to the current scheduler task. @@ -720,6 +714,7 @@ public abstract class Monitor // Start the scheduler. // cleanupFutures(); + schedulerTask.setMonitorTask(new MonitorTask()); schedulerFuture = scheduler.schedule(schedulerTask, getGranularityPeriod(), TimeUnit.MILLISECONDS); @@ -1468,7 +1463,7 @@ public abstract class Monitor */ private class SchedulerTask implements Runnable { - private Runnable task = null; + private MonitorTask task; /* * ------------------------------------------ @@ -1476,7 +1471,16 @@ public abstract class Monitor * ------------------------------------------ */ - public SchedulerTask(Runnable task) { + public SchedulerTask() { + } + + /* + * ------------------------------------------ + * GETTERS/SETTERS + * ------------------------------------------ + */ + + public void setMonitorTask(MonitorTask task) { this.task = task; } @@ -1488,7 +1492,7 @@ public abstract class Monitor public void run() { synchronized (Monitor.this) { - Monitor.this.monitorFuture = executor.submit(task); + Monitor.this.monitorFuture = task.submit(); } } } @@ -1501,6 +1505,8 @@ public abstract class Monitor */ private class MonitorTask implements Runnable { + private ThreadPoolExecutor executor; + /* * ------------------------------------------ * CONSTRUCTORS @@ -1508,6 +1514,38 @@ public abstract class Monitor */ public MonitorTask() { + // Find out if there's already an existing executor for the calling + // thread and reuse it. Otherwise, create a new one and store it in + // the executors map. If there is a SecurityManager, the group of + // System.getSecurityManager() is used, else the group of the thread + // instantiating this MonitorTask, i.e. the group of the thread that + // calls "Monitor.start()". + SecurityManager s = System.getSecurityManager(); + ThreadGroup group = (s != null) ? s.getThreadGroup() : + Thread.currentThread().getThreadGroup(); + synchronized (executorsLock) { + for (ThreadPoolExecutor e : executors.keySet()) { + DaemonThreadFactory tf = + (DaemonThreadFactory) e.getThreadFactory(); + ThreadGroup tg = tf.getThreadGroup(); + if (tg == group) { + executor = e; + break; + } + } + if (executor == null) { + executor = new ThreadPoolExecutor( + maximumPoolSize, + maximumPoolSize, + 60L, + TimeUnit.SECONDS, + new LinkedBlockingQueue(), + new DaemonThreadFactory("ThreadGroup<" + + group.getName() + "> Executor", group)); + executor.allowCoreThreadTimeOut(true); + executors.put(executor, null); + } + } } /* @@ -1516,6 +1554,10 @@ public abstract class Monitor * ------------------------------------------ */ + public Future submit() { + return executor.submit(this); + } + public void run() { final ScheduledFuture sf; synchronized (Monitor.this) { @@ -1574,6 +1616,15 @@ public abstract class Monitor namePrefix = "JMX Monitor " + poolName + " Pool [Thread-"; } + public DaemonThreadFactory(String poolName, ThreadGroup threadGroup) { + group = threadGroup; + namePrefix = "JMX Monitor " + poolName + " Pool [Thread-"; + } + + public ThreadGroup getThreadGroup() { + return group; + } + public Thread newThread(Runnable r) { Thread t = new Thread(group, r, From 576a962dcb64912573e7298c03ef8d66cce17ead Mon Sep 17 00:00:00 2001 From: Daniel Fuchs Date: Mon, 9 Mar 2009 23:50:11 +0100 Subject: [PATCH 21/23] 6721651: Security problem with out-of-the-box management Reviewed-by: emcmanus, lmalvent --- .../security/MBeanServerAccessController.java | 52 ++- .../MBeanServerFileAccessController.java | 376 +++++++++++++++--- jdk/src/share/lib/management/jmxremote.access | 47 ++- 3 files changed, 389 insertions(+), 86 deletions(-) diff --git a/jdk/src/share/classes/com/sun/jmx/remote/security/MBeanServerAccessController.java b/jdk/src/share/classes/com/sun/jmx/remote/security/MBeanServerAccessController.java index d75f829c321..71e6f6ff019 100644 --- a/jdk/src/share/classes/com/sun/jmx/remote/security/MBeanServerAccessController.java +++ b/jdk/src/share/classes/com/sun/jmx/remote/security/MBeanServerAccessController.java @@ -111,6 +111,22 @@ public abstract class MBeanServerAccessController */ protected abstract void checkWrite(); + /** + * Check if the caller can create the named class. The default + * implementation of this method calls {@link #checkWrite()}. + */ + protected void checkCreate(String className) { + checkWrite(); + } + + /** + * Check if the caller can unregister the named MBean. The default + * implementation of this method calls {@link #checkWrite()}. + */ + protected void checkUnregister(ObjectName name) { + checkWrite(); + } + //-------------------------------------------- //-------------------------------------------- // @@ -148,7 +164,7 @@ public abstract class MBeanServerAccessController } /** - * Call checkWrite(), then forward this method to the + * Call checkCreate(className), then forward this method to the * wrapped object. */ public ObjectInstance createMBean(String className, ObjectName name) @@ -158,7 +174,7 @@ public abstract class MBeanServerAccessController MBeanRegistrationException, MBeanException, NotCompliantMBeanException { - checkWrite(); + checkCreate(className); SecurityManager sm = System.getSecurityManager(); if (sm == null) { Object object = getMBeanServer().instantiate(className); @@ -170,7 +186,7 @@ public abstract class MBeanServerAccessController } /** - * Call checkWrite(), then forward this method to the + * Call checkCreate(className), then forward this method to the * wrapped object. */ public ObjectInstance createMBean(String className, ObjectName name, @@ -181,7 +197,7 @@ public abstract class MBeanServerAccessController MBeanRegistrationException, MBeanException, NotCompliantMBeanException { - checkWrite(); + checkCreate(className); SecurityManager sm = System.getSecurityManager(); if (sm == null) { Object object = getMBeanServer().instantiate(className, @@ -196,7 +212,7 @@ public abstract class MBeanServerAccessController } /** - * Call checkWrite(), then forward this method to the + * Call checkCreate(className), then forward this method to the * wrapped object. */ public ObjectInstance createMBean(String className, @@ -209,7 +225,7 @@ public abstract class MBeanServerAccessController MBeanException, NotCompliantMBeanException, InstanceNotFoundException { - checkWrite(); + checkCreate(className); SecurityManager sm = System.getSecurityManager(); if (sm == null) { Object object = getMBeanServer().instantiate(className, @@ -222,7 +238,7 @@ public abstract class MBeanServerAccessController } /** - * Call checkWrite(), then forward this method to the + * Call checkCreate(className), then forward this method to the * wrapped object. */ public ObjectInstance createMBean(String className, @@ -237,7 +253,7 @@ public abstract class MBeanServerAccessController MBeanException, NotCompliantMBeanException, InstanceNotFoundException { - checkWrite(); + checkCreate(className); SecurityManager sm = System.getSecurityManager(); if (sm == null) { Object object = getMBeanServer().instantiate(className, @@ -394,45 +410,45 @@ public abstract class MBeanServerAccessController } /** - * Call checkWrite(), then forward this method to the + * Call checkCreate(className), then forward this method to the * wrapped object. */ public Object instantiate(String className) throws ReflectionException, MBeanException { - checkWrite(); + checkCreate(className); return getMBeanServer().instantiate(className); } /** - * Call checkWrite(), then forward this method to the + * Call checkCreate(className), then forward this method to the * wrapped object. */ public Object instantiate(String className, Object params[], String signature[]) throws ReflectionException, MBeanException { - checkWrite(); + checkCreate(className); return getMBeanServer().instantiate(className, params, signature); } /** - * Call checkWrite(), then forward this method to the + * Call checkCreate(className), then forward this method to the * wrapped object. */ public Object instantiate(String className, ObjectName loaderName) throws ReflectionException, MBeanException, InstanceNotFoundException { - checkWrite(); + checkCreate(className); return getMBeanServer().instantiate(className, loaderName); } /** - * Call checkWrite(), then forward this method to the + * Call checkCreate(className), then forward this method to the * wrapped object. */ public Object instantiate(String className, ObjectName loaderName, Object params[], String signature[]) throws ReflectionException, MBeanException, InstanceNotFoundException { - checkWrite(); + checkCreate(className); return getMBeanServer().instantiate(className, loaderName, params, signature); } @@ -579,12 +595,12 @@ public abstract class MBeanServerAccessController } /** - * Call checkWrite(), then forward this method to the + * Call checkUnregister(), then forward this method to the * wrapped object. */ public void unregisterMBean(ObjectName name) throws InstanceNotFoundException, MBeanRegistrationException { - checkWrite(); + checkUnregister(name); getMBeanServer().unregisterMBean(name); } diff --git a/jdk/src/share/classes/com/sun/jmx/remote/security/MBeanServerFileAccessController.java b/jdk/src/share/classes/com/sun/jmx/remote/security/MBeanServerFileAccessController.java index 8bca71dbc62..01bb6108e43 100644 --- a/jdk/src/share/classes/com/sun/jmx/remote/security/MBeanServerFileAccessController.java +++ b/jdk/src/share/classes/com/sun/jmx/remote/security/MBeanServerFileAccessController.java @@ -31,11 +31,17 @@ import java.security.AccessControlContext; import java.security.AccessController; import java.security.Principal; import java.security.PrivilegedAction; -import java.util.Collection; +import java.util.ArrayList; +import java.util.HashMap; import java.util.Iterator; +import java.util.List; +import java.util.Map; import java.util.Properties; import java.util.Set; +import java.util.StringTokenizer; +import java.util.regex.Pattern; import javax.management.MBeanServer; +import javax.management.ObjectName; import javax.security.auth.Subject; /** @@ -46,7 +52,8 @@ import javax.security.auth.Subject; * not allowed; in this case the request is not forwarded to the * wrapped object.

* - *

This class implements the {@link #checkRead()} and {@link #checkWrite()} + *

This class implements the {@link #checkRead()}, {@link #checkWrite()}, + * {@link #checkCreate(String)}, and {@link #checkUnregister(ObjectName)} * methods based on an access level properties file containing username/access * level pairs. The set of username/access level pairs is passed either as a * filename which denotes a properties file on disk, or directly as an instance @@ -56,14 +63,50 @@ import javax.security.auth.Subject; * has exactly one access level. The same access level can be shared by several * usernames.

* - *

The supported access level values are readonly and - * readwrite.

+ *

The supported access level values are {@code readonly} and + * {@code readwrite}. The {@code readwrite} access level can be + * qualified by one or more clauses, where each clause looks + * like create classNamePattern or {@code + * unregister}. For example:

+ * + * 
+ * monitorRole  readonly
+ * controlRole  readwrite \
+ *              create javax.management.timer.*,javax.management.monitor.* \
+ *              unregister
+ *
+ * + *

(The continuation lines with {@code \} come from the parser for + * Properties files.)

*/ public class MBeanServerFileAccessController extends MBeanServerAccessController { - public static final String READONLY = "readonly"; - public static final String READWRITE = "readwrite"; + static final String READONLY = "readonly"; + static final String READWRITE = "readwrite"; + + static final String CREATE = "create"; + static final String UNREGISTER = "unregister"; + + private enum AccessType {READ, WRITE, CREATE, UNREGISTER}; + + private static class Access { + final boolean write; + final String[] createPatterns; + private boolean unregister; + + Access(boolean write, boolean unregister, List createPatternList) { + this.write = write; + int npats = (createPatternList == null) ? 0 : createPatternList.size(); + if (npats == 0) + this.createPatterns = NO_STRINGS; + else + this.createPatterns = createPatternList.toArray(new String[npats]); + this.unregister = unregister; + } + + private final String[] NO_STRINGS = new String[0]; + } /** *

Create a new MBeanServerAccessController that forwards all the @@ -87,8 +130,8 @@ public class MBeanServerFileAccessController throws IOException { super(); this.accessFileName = accessFileName; - props = propertiesFromFile(accessFileName); - checkValues(props); + Properties props = propertiesFromFile(accessFileName); + parseProperties(props); } /** @@ -123,14 +166,14 @@ public class MBeanServerFileAccessController * #setMBeanServer} method after doing access checks based on read and * write permissions.

* - *

This instance is initialized from the specified properties instance. - * This constructor makes a copy of the properties instance using its - * clone method and it is the copy that is consulted to check - * the username and access level of an incoming connection. The original - * properties object can be modified without affecting the copy. If the - * {@link #refresh} method is then called, the - * MBeanServerFileAccessController will make a new copy of the - * properties object at that time.

+ *

This instance is initialized from the specified properties + * instance. This constructor makes a copy of the properties + * instance and it is the copy that is consulted to check the + * username and access level of an incoming connection. The + * original properties object can be modified without affecting + * the copy. If the {@link #refresh} method is then called, the + * MBeanServerFileAccessController will make a new + * copy of the properties object at that time.

* * @param accessFileProps properties list containing the username/access * level entries. @@ -145,8 +188,7 @@ public class MBeanServerFileAccessController if (accessFileProps == null) throw new IllegalArgumentException("Null properties"); originalProps = accessFileProps; - props = (Properties) accessFileProps.clone(); - checkValues(props); + parseProperties(accessFileProps); } /** @@ -155,14 +197,14 @@ public class MBeanServerFileAccessController * #setMBeanServer} method after doing access checks based on read and * write permissions.

* - *

This instance is initialized from the specified properties instance. - * This constructor makes a copy of the properties instance using its - * clone method and it is the copy that is consulted to check - * the username and access level of an incoming connection. The original - * properties object can be modified without affecting the copy. If the - * {@link #refresh} method is then called, the - * MBeanServerFileAccessController will make a new copy of the - * properties object at that time.

+ *

This instance is initialized from the specified properties + * instance. This constructor makes a copy of the properties + * instance and it is the copy that is consulted to check the + * username and access level of an incoming connection. The + * original properties object can be modified without affecting + * the copy. If the {@link #refresh} method is then called, the + * MBeanServerFileAccessController will make a new + * copy of the properties object at that time.

* * @param accessFileProps properties list containing the username/access * level entries. @@ -184,16 +226,36 @@ public class MBeanServerFileAccessController * Check if the caller can do read operations. This method does * nothing if so, otherwise throws SecurityException. */ + @Override public void checkRead() { - checkAccessLevel(READONLY); + checkAccess(AccessType.READ, null); } /** * Check if the caller can do write operations. This method does * nothing if so, otherwise throws SecurityException. */ + @Override public void checkWrite() { - checkAccessLevel(READWRITE); + checkAccess(AccessType.WRITE, null); + } + + /** + * Check if the caller can create MBeans or instances of the given class. + * This method does nothing if so, otherwise throws SecurityException. + */ + @Override + public void checkCreate(String className) { + checkAccess(AccessType.CREATE, className); + } + + /** + * Check if the caller can do unregister operations. This method does + * nothing if so, otherwise throws SecurityException. + */ + @Override + public void checkUnregister(ObjectName name) { + checkAccess(AccessType.UNREGISTER, null); } /** @@ -218,14 +280,13 @@ public class MBeanServerFileAccessController * @exception IllegalArgumentException if any of the supplied access * level values differs from "readonly" or "readwrite". */ - public void refresh() throws IOException { - synchronized (props) { - if (accessFileName == null) - props = (Properties) originalProps.clone(); - else - props = propertiesFromFile(accessFileName); - checkValues(props); - } + public synchronized void refresh() throws IOException { + Properties props; + if (accessFileName == null) + props = (Properties) originalProps; + else + props = propertiesFromFile(accessFileName); + parseProperties(props); } private static Properties propertiesFromFile(String fname) @@ -240,7 +301,7 @@ public class MBeanServerFileAccessController } } - private void checkAccessLevel(String accessLevel) { + private synchronized void checkAccess(AccessType requiredAccess, String arg) { final AccessControlContext acc = AccessController.getContext(); final Subject s = AccessController.doPrivileged(new PrivilegedAction() { @@ -250,39 +311,234 @@ public class MBeanServerFileAccessController }); if (s == null) return; /* security has not been enabled */ final Set principals = s.getPrincipals(); + String newPropertyValue = null; for (Iterator i = principals.iterator(); i.hasNext(); ) { final Principal p = (Principal) i.next(); - String grantedAccessLevel; - synchronized (props) { - grantedAccessLevel = props.getProperty(p.getName()); - } - if (grantedAccessLevel != null) { - if (accessLevel.equals(READONLY) && - (grantedAccessLevel.equals(READONLY) || - grantedAccessLevel.equals(READWRITE))) - return; - if (accessLevel.equals(READWRITE) && - grantedAccessLevel.equals(READWRITE)) + Access access = accessMap.get(p.getName()); + if (access != null) { + boolean ok; + switch (requiredAccess) { + case READ: + ok = true; // all access entries imply read + break; + case WRITE: + ok = access.write; + break; + case UNREGISTER: + ok = access.unregister; + if (!ok && access.write) + newPropertyValue = "unregister"; + break; + case CREATE: + ok = checkCreateAccess(access, arg); + if (!ok && access.write) + newPropertyValue = "create " + arg; + break; + default: + throw new AssertionError(); + } + if (ok) return; } } - throw new SecurityException("Access denied! Invalid access level for " + - "requested MBeanServer operation."); + SecurityException se = new SecurityException("Access denied! Invalid " + + "access level for requested MBeanServer operation."); + // Add some more information to help people with deployments that + // worked before we required explicit create clauses. We're not giving + // any information to the bad guys, other than that the access control + // is based on a file, which they could have worked out from the stack + // trace anyway. + if (newPropertyValue != null) { + SecurityException se2 = new SecurityException("Access property " + + "for this identity should be similar to: " + READWRITE + + " " + newPropertyValue); + se.initCause(se2); + } + throw se; } - private void checkValues(Properties props) { - Collection c = props.values(); - for (Iterator i = c.iterator(); i.hasNext(); ) { - final String accessLevel = (String) i.next(); - if (!accessLevel.equals(READONLY) && - !accessLevel.equals(READWRITE)) { - throw new IllegalArgumentException( - "Syntax error in access level entry [" + accessLevel + "]"); - } + private static boolean checkCreateAccess(Access access, String className) { + for (String classNamePattern : access.createPatterns) { + if (classNameMatch(classNamePattern, className)) + return true; + } + return false; + } + + private static boolean classNameMatch(String pattern, String className) { + // We studiously avoided regexes when parsing the properties file, + // because that is done whenever the VM is started with the + // appropriate -Dcom.sun.management options, even if nobody ever + // creates an MBean. We don't want to incur the overhead of loading + // all the regex code whenever those options are specified, but if we + // get as far as here then the VM is already running and somebody is + // doing the very unusual operation of remotely creating an MBean. + // Because that operation is so unusual, we don't try to optimize + // by hand-matching or by caching compiled Pattern objects. + StringBuilder sb = new StringBuilder(); + StringTokenizer stok = new StringTokenizer(pattern, "*", true); + while (stok.hasMoreTokens()) { + String tok = stok.nextToken(); + if (tok.equals("*")) + sb.append("[^.]*"); + else + sb.append(Pattern.quote(tok)); + } + return className.matches(sb.toString()); + } + + private void parseProperties(Properties props) { + this.accessMap = new HashMap(); + for (Map.Entry entry : props.entrySet()) { + String identity = (String) entry.getKey(); + String accessString = (String) entry.getValue(); + Access access = Parser.parseAccess(identity, accessString); + accessMap.put(identity, access); } } - private Properties props; + private static class Parser { + private final static int EOS = -1; // pseudo-codepoint "end of string" + static { + assert !Character.isWhitespace(EOS); + } + + private final String identity; // just for better error messages + private final String s; // the string we're parsing + private final int len; // s.length() + private int i; + private int c; + // At any point, either c is s.codePointAt(i), or i == len and + // c is EOS. We use int rather than char because it is conceivable + // (if unlikely) that a classname in a create clause might contain + // "supplementary characters", the ones that don't fit in the original + // 16 bits for Unicode. + + private Parser(String identity, String s) { + this.identity = identity; + this.s = s; + this.len = s.length(); + this.i = 0; + if (i < len) + this.c = s.codePointAt(i); + else + this.c = EOS; + } + + static Access parseAccess(String identity, String s) { + return new Parser(identity, s).parseAccess(); + } + + private Access parseAccess() { + skipSpace(); + String type = parseWord(); + Access access; + if (type.equals(READONLY)) + access = new Access(false, false, null); + else if (type.equals(READWRITE)) + access = parseReadWrite(); + else { + throw syntax("Expected " + READONLY + " or " + READWRITE + + ": " + type); + } + if (c != EOS) + throw syntax("Extra text at end of line"); + return access; + } + + private Access parseReadWrite() { + List createClasses = new ArrayList(); + boolean unregister = false; + while (true) { + skipSpace(); + if (c == EOS) + break; + String type = parseWord(); + if (type.equals(UNREGISTER)) + unregister = true; + else if (type.equals(CREATE)) + parseCreate(createClasses); + else + throw syntax("Unrecognized keyword " + type); + } + return new Access(true, unregister, createClasses); + } + + private void parseCreate(List createClasses) { + while (true) { + skipSpace(); + createClasses.add(parseClassName()); + skipSpace(); + if (c == ',') + next(); + else + break; + } + } + + private String parseClassName() { + // We don't check that classname components begin with suitable + // characters (so we accept 1.2.3 for example). This means that + // there are only two states, which we can call dotOK and !dotOK + // according as a dot (.) is legal or not. Initially we're in + // !dotOK since a classname can't start with a dot; after a dot + // we're in !dotOK again; and after any other characters we're in + // dotOK. The classname is only accepted if we end in dotOK, + // so we reject an empty name or a name that ends with a dot. + final int start = i; + boolean dotOK = false; + while (true) { + if (c == '.') { + if (!dotOK) + throw syntax("Bad . in class name"); + dotOK = false; + } else if (c == '*' || Character.isJavaIdentifierPart(c)) + dotOK = true; + else + break; + next(); + } + String className = s.substring(start, i); + if (!dotOK) + throw syntax("Bad class name " + className); + return className; + } + + // Advance c and i to the next character, unless already at EOS. + private void next() { + if (c != EOS) { + i += Character.charCount(c); + if (i < len) + c = s.codePointAt(i); + else + c = EOS; + } + } + + private void skipSpace() { + while (Character.isWhitespace(c)) + next(); + } + + private String parseWord() { + skipSpace(); + if (c == EOS) + throw syntax("Expected word at end of line"); + final int start = i; + while (c != EOS && !Character.isWhitespace(c)) + next(); + String word = s.substring(start, i); + skipSpace(); + return word; + } + + private IllegalArgumentException syntax(String msg) { + return new IllegalArgumentException( + msg + " [" + identity + " " + s + "]"); + } + } + + private Map accessMap; private Properties originalProps; private String accessFileName; } diff --git a/jdk/src/share/lib/management/jmxremote.access b/jdk/src/share/lib/management/jmxremote.access index 765f118a364..ce80b47a1a8 100644 --- a/jdk/src/share/lib/management/jmxremote.access +++ b/jdk/src/share/lib/management/jmxremote.access @@ -8,7 +8,7 @@ # passwords. To be functional, a role must have an entry in # both the password and the access files. # -# Default location of this file is $JRE/lib/management/jmxremote.access +# The default location of this file is $JRE/lib/management/jmxremote.access # You can specify an alternate location by specifying a property in # the management config file $JRE/lib/management/management.properties # (See that file for details) @@ -16,7 +16,7 @@ # The file format for password and access files is syntactically the same # as the Properties file format. The syntax is described in the Javadoc # for java.util.Properties.load. -# Typical access file has multiple lines, where each line is blank, +# A typical access file has multiple lines, where each line is blank, # a comment (like this one), or an access control entry. # # An access control entry consists of a role name, and an @@ -29,10 +29,38 @@ # role can read measurements but cannot perform any action # that changes the environment of the running program. # "readwrite" grants access to read and write attributes of MBeans, -# to invoke operations on them, and to create or remove them. -# This access should be granted to only trusted clients, -# since they can potentially interfere with the smooth -# operation of a running program +# to invoke operations on them, and optionally +# to create or remove them. This access should be granted +# only to trusted clients, since they can potentially +# interfere with the smooth operation of a running program. +# +# The "readwrite" access level can optionally be followed by the "create" and/or +# "unregister" keywords. The "unregister" keyword grants access to unregister +# (delete) MBeans. The "create" keyword grants access to create MBeans of a +# particular class or of any class matching a particular pattern. Access +# should only be granted to create MBeans of known and trusted classes. +# +# For example, the following entry would grant readwrite access +# to "controlRole", as well as access to create MBeans of the class +# javax.management.monitor.CounterMonitor and to unregister any MBean: +# controlRole readwrite \ +# create javax.management.monitor.CounterMonitorMBean \ +# unregister +# or equivalently: +# controlRole readwrite unregister create javax.management.monitor.CounterMBean +# +# The following entry would grant readwrite access as well as access to create +# MBeans of any class in the packages javax.management.monitor and +# javax.management.timer: +# controlRole readwrite \ +# create javax.management.monitor.*,javax.management.timer.* \ +# unregister +# +# The \ character is defined in the Properties file syntax to allow continuation +# lines as shown here. A * in a class pattern matches a sequence of characters +# other than dot (.), so javax.management.monitor.* matches +# javax.management.monitor.CounterMonitor but not +# javax.management.monitor.foo.Bar. # # A given role should have at most one entry in this file. If a role # has no entry, it has no access. @@ -42,7 +70,10 @@ # # Default access control entries: # o The "monitorRole" role has readonly access. -# o The "controlRole" role has readwrite access. +# o The "controlRole" role has readwrite access and can create the standard +# Timer and Monitor MBeans defined by the JMX API. monitorRole readonly -controlRole readwrite +controlRole readwrite \ + create javax.management.monitor.*,javax.management.timer.* \ + unregister From a0ec52da3321d09d06b2b59adac591d63d9330cc Mon Sep 17 00:00:00 2001 From: Michael McMahon Date: Tue, 10 Mar 2009 03:18:22 -0700 Subject: [PATCH 22/23] 6630639: lightweight HttpServer leaks file descriptors on no-data connections Not cleaning up no-data connections properly Reviewed-by: chegar --- jdk/src/share/classes/sun/net/httpserver/Request.java | 3 +++ jdk/src/share/classes/sun/net/httpserver/ServerImpl.java | 3 +++ 2 files changed, 6 insertions(+) diff --git a/jdk/src/share/classes/sun/net/httpserver/Request.java b/jdk/src/share/classes/sun/net/httpserver/Request.java index bdafe663c1b..02d240c989c 100644 --- a/jdk/src/share/classes/sun/net/httpserver/Request.java +++ b/jdk/src/share/classes/sun/net/httpserver/Request.java @@ -52,6 +52,9 @@ class Request { os = rawout; do { startLine = readLine(); + if (startLine == null) { + return; + } /* skip blank lines */ } while (startLine.equals ("")); } diff --git a/jdk/src/share/classes/sun/net/httpserver/ServerImpl.java b/jdk/src/share/classes/sun/net/httpserver/ServerImpl.java index 104bdb0180b..919f7ce8ab6 100644 --- a/jdk/src/share/classes/sun/net/httpserver/ServerImpl.java +++ b/jdk/src/share/classes/sun/net/httpserver/ServerImpl.java @@ -441,6 +441,7 @@ class ServerImpl implements TimeSource { rawin = sslStreams.getInputStream(); rawout = sslStreams.getOutputStream(); engine = sslStreams.getSSLEngine(); + connection.sslStreams = sslStreams; } else { rawin = new BufferedInputStream( new Request.ReadStream ( @@ -450,6 +451,8 @@ class ServerImpl implements TimeSource { ServerImpl.this, chan ); } + connection.raw = rawin; + connection.rawout = rawout; } Request req = new Request (rawin, rawout); requestLine = req.requestLine(); From 006e84fc77a582552e71a888db1be31407b783c7 Mon Sep 17 00:00:00 2001 From: Vinnie Ryan Date: Tue, 10 Mar 2009 18:43:00 +0000 Subject: [PATCH 23/23] 6737315: LDAP serialized data vulnerability Reviewed-by: alanb --- .../com/sun/jndi/ldap/VersionHelper12.java | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/jdk/src/share/classes/com/sun/jndi/ldap/VersionHelper12.java b/jdk/src/share/classes/com/sun/jndi/ldap/VersionHelper12.java index a4131166d30..0f6a70e04c9 100644 --- a/jdk/src/share/classes/com/sun/jndi/ldap/VersionHelper12.java +++ b/jdk/src/share/classes/com/sun/jndi/ldap/VersionHelper12.java @@ -1,5 +1,5 @@ /* - * Copyright 1999 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 1999-2009 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -33,12 +33,33 @@ import java.security.PrivilegedAction; final class VersionHelper12 extends VersionHelper { + // System property to control whether classes may be loaded from an + // arbitrary URL code base. + private static final String TRUST_URL_CODEBASE_PROPERTY = + "com.sun.jndi.ldap.object.trustURLCodebase"; + + // Determine whether classes may be loaded from an arbitrary URL code base. + private static final String trustURLCodebase = + AccessController.doPrivileged( + new PrivilegedAction() { + public String run() { + return System.getProperty(TRUST_URL_CODEBASE_PROPERTY, + "false"); + } + } + ); + VersionHelper12() {} // Disallow external from creating one of these. ClassLoader getURLClassLoader(String[] url) throws MalformedURLException { ClassLoader parent = getContextClassLoader(); - if (url != null) { + /* + * Classes may only be loaded from an arbitrary URL code base when + * the system property com.sun.jndi.ldap.object.trustURLCodebase + * has been set to "true". + */ + if (url != null && "true".equalsIgnoreCase(trustURLCodebase)) { return URLClassLoader.newInstance(getUrlArray(url), parent); } else { return parent;