Detecting SSLv3 in Java

Posted by Jon, Comments

Our SAST product, Security Advisor, recently released a couple of new checkers and updated a couple existing ones. One of the checkers, RISKY_CRYPTO, is now looking for SSLv3. SSLv3 should be considered beyond deprecated because of POODLE, so its use is truly risky at this point. The checker looks for its use implicitly (e.g. some JRE defaults) or explicitly, in either client or server sockets.

An example defect is in org.alfresco.encryption.ssl.AuthSSLProtocolSocketFactory.createSocket, from the Alfresco project. The new version of the analysis flags a defect on line 175, when the socket is bound. Implicitly, the SSLv3 protocol is allowed in the JVM, so this socket potentially exposes itself to the POODLE vulnerability. (If CBC isn't allowed, then this would be a false positive.)



Not bad remediation advice, eh?

As a recovering security consultant, I really hated any tool that reported general crypto usage of say MD5 or something. While RISKY_CRYPTO does this, because sadly people do ask for us to look for this, we're also releasing a smarter crypto checker called WEAK_PASSWORD_HASH.

Take the method com.laoer.bbscs.comm.Util.hash from Java web application called BBS Community System.

public synchronized static final String hash(String data) {
    if (digest == null) {
        try {
            digest = MessageDigest.getInstance("MD5");
        } catch (NoSuchAlgorithmException nsae) {
            System.err
                    .println("Failed to load the MD5 MessageDigest. " + "We will be unable to function normally.");
            nsae.printStackTrace();
        }
    }
    // Now, compute hash.
    digest.update(data.getBytes());
    return encodeHex(digest.digest());
}

The RISKY_CRYPTO checker will flag the "MD5" as being bad, mmmkay. The issue here isn't that MD5 is being used, it's that it's being used to hash a password. Now pointing out MD5 might be good enough for a security professional. It's like blood in the water. However, just stating XYZ algorithm is in use isn't necessarily evil. Developers want more. If you're a security person, be ready to answer: "why is it bad in this context, in this piece of code?"

Glossing over some of the details, the new WEAK_PASSWORD_HASH checker flags data it thinks is a password. It then tracks the password data flow until it reaches a hashing sink it thinks is not adequate. (There's a bit about salts in there I'm skipping, but you get the idea.)

Case in point, WEAK_PASSWORD_HASH correctly infers the Struts 2 entry point com.laoer.bbscs.web.action.Cpasswd.setOldPassword as a source of password data. It tracks the data flow of this field to line 99:

UserInfo ui = this.getUserService().findUserInfoById(this.getUserSession().getId());
if (ui != null) {
    String op = Util.hash(this.getOldpasswd());
    if (!op.equals(ui.getRePasswd())) {
        this.addActionError(this.getText("error.changepasswdold"));
        return INPUT;
    }

... where the unsafe Util.hash method is called. Now that's a defect I'd rather see than RISKY_CRYPTO, or whatever your SAST tool's checker is called, flagging the use of MD5. Now, your developers have the answer to their question without involving anyone. Devs like that.

Comments!