Skip to end of metadata
Go to start of metadata

Hard coding sensitive information, such as passwords, server IP addresses, and encryption keys can expose the information to attackers. Anyone who has access to the class files can decompile them and discover the sensitive information. Leaking data protected by International Traffic in Arms Regulations (ITAR) or the Health Insurance Portability and Accountability Act (HIPAA) can also have legal consequences. Consequently, programs must not hard code sensitive information.

Hard coding sensitive information also increases the need to manage and accommodate changes to the code. For example, changing a hard-coded password in a deployed program may require distribution of a patch [Chess 2007].

Noncompliant Code Example

This noncompliant code example includes a hard-coded server IP address in a constant String:

class IPaddress {
  String ipAddress = new String("172.16.254.1");
  public static void main(String[] args) {
    //...
  }
}

A malicious user can use the javap -c IPaddress command to disassemble the class and discover the hard-coded server IP address. The output of the disassembler reveals the server IP address 172.16.254.1 in clear text:

Compiled from "IPaddress.java"
class IPaddress extends java.lang.Object{
java.lang.String ipAddress;

IPaddress();
  Code:
   0:     aload_0
   1:     invokespecial     #1; //Method java/lang/Object."<init>":()V
   4:     aload_0
   5:     new   #2; //class java/lang/String
   8:     dup
   9:     ldc   #3; //String 172.16.254.1
   11:    invokespecial     #4; //Method java/lang/String."<init>":(Ljava/lang/String;)V
   14:    putfield    #5; //Field ipAddress:Ljava/lang/String;
   17:    return

public static void main(java.lang.String[]);
  Code:
   0:     return

}

Compliant Solution

This compliant solution retrieves the server IP address from an external file located in a secure directory, as recommended by FIO00-J. Do not operate on files in shared directories. It reads the file in compliance with FIO10-J. Ensure the array is filled when using read() to fill an array. Exposure of the IP address is further limited by storing it in a char array rather than a java.lang.String, and by clearing the server IP address from memory immediately after use.

class IPaddress {
  public static void main(String[] args) throws IOException {
    char[] ipAddress = new char[100];
    int offset = 0;
    int charsRead = 0;
    BufferedReader br = null;
    try {
      br = new BufferedReader(new InputStreamReader(
             new FileInputStream("serveripaddress.txt")));
      while ((charsRead = br.read(ipAddress, offset, ipAddress.length - offset))
          != -1) {
        offset += charsRead;
        if (offset >= ipAddress.length) {
          break;
        }
      }
      
      // ... Work with IP address

    } finally {
      Arrays.fill(ipAddress,  (byte) 0);
      br.close();
    }
  }
}

To further limit the exposure time of the sensitive server IP address, replace BufferedReader with a direct native input/output (NIO) buffer, which can be cleared immediately after use.

Noncompliant Code Example (Hard-Coded Database Password)

The user name and password fields in the SQL connection request are hard coded in this noncompliant code example:

public final Connection getConnection() throws SQLException {
  return DriverManager.getConnection(
      "jdbc:mysql://localhost/dbName", 
      "username", "password");
}

Note that the one- and two-argument java.sql.DriverManager.getConnection() methods can also be used incorrectly.

Compliant Solution

This compliant solution reads the user name and password from a configuration file located in a secure directory:

public final Connection getConnection() throws SQLException {
  String username;
  String password;
  // Username and password are read at runtime from a secure config file
  return DriverManager.getConnection(
      "jdbc:mysql://localhost/dbName", username, password);
}

It is also permissible to prompt the user for the user name and password at runtime.

When possible, sensitive information such as passwords should be stored in character arrays rather than strings because the Java Virtual Machine may retain strings long after they are no longer needed. However, this example uses strings because DriverManager.getConnection() requires them.

Risk Assessment

Hard coding sensitive information exposes that information to attackers. The severity of this rule can vary depending on the kind of information that is disclosed. Frequently, the information disclosed is password or key information, which can lead to remote exploitation. Consequently, a high severity rating is given but may be adjusted downwards according to the nature of the sensitive data

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

MSC03-J

High

Probable

Medium

P12

L1

Automated Detection

ToolVersionCheckerDescription
CodeSonar4.2

FB.SECURITY.DMI_CONSTANT_DB_PASSWORD

FB.SECURITY.DMI_EMPTY_DB_PASSWORD

Hardcoded constant database password

Empty database password

Coverity7.5

HARDCODED_CREDENTIALS
CONFIG
FB.DMI_CONSTANT_DB_ PASSWORD
FB.DMI_EMPTY_DB_PASSWORD

Implemented
Fortify1.0

Password_Management
Password_Management__Hardcoded_Password

Partially implemented
Parasoft Jtest 10.3 SECURITY.WSC.HCCS, SECURITY.WSC.HCCK, SECURITY.WSC.AHCAImplemented
PMD1.0AvoidUsingHardCodedIPPartially implemented
SonarQube6.7S1313
S2068
Partially implemented

Related Vulnerabilities

GERONIMO-2925 describes a vulnerability in the WAS CE tool, which is based on Apache Geronimo. It uses the Advanced Encryption Standard (AES) to encrypt passwords but uses a hard-coded key that is identical for all the WAS CE server instances. Consequently, anyone who can download the software is provided with the key to every instance of the tool. This vulnerability was resolved by having each new installation of the tool generate its own unique key and use it from that time on.

Related Guidelines

Android Implementation Details

Hard-coded information can be easily obtained on Android by using the apktool to decompile an application or by using dex2jar to convert a dex file to a jar file.

Bibliography

[Chess 2007]

Section 11.2, "Outbound Passwords: Keep Passwords out of Source Code"

[Fortify 2008]

"Unsafe Mobile Code: Database Access"

[Gong 2003]

Section 9.4, "Private Object State and Object Immutability"

 


  

20 Comments

  1. We could possibly have another rule - "Do not store sensitive information in immutable objects", such as a password in a String object (as shown in this rule). The NCCE/CS will be the same as this rule so am unsure if simply replicating it will be a good idea.

    1. Clearing after use isn't as useful as it might appear.

      • Objects are copied within memory, so the original copy may not have been cleared.
      • Swapping and hibernation will write it to disk, where it can remain for months after multiple reboots.
      • In the compliant example, the password is still in the BufferedReader, in the OS cache, and in places where the bytes are being decoded into chars.
      • There are control flows that bypass the clear. For instance from reflections.
      • Java's strong typing should protect you from reading the data from elsewhere (although calling native code may still manage it).
      1. Using mutable types will be somewhat better but if there are copies in the memory then they won't be cleared. Though this could limit the window of vulnerability at a low cost, I concur that we can't use it as a CS as you suggest, as it is not an exemplar.

        We can't avoid swapping out either unless we use some native code and never bring java objects in the picture. Is this a better idea? Another way I can think of is to only store one-way hashes and always compute a hash on the user's input (use a salt) and then compare, but it won't be applicable for all scenarios (storing credit card numbers temporarily etc.). In your opinion, what should the compliant solution be? From the Sun Forums, it seems that every now and then some programmer comes up with this issue, but gets contradictory answers.

        This particular rule is about not hardcoding sensitive info, but all these comments will be addressed in a separate rule/recommendation with currently known best practices. Thanks!

        1. The best you could do is configure the machine not to swap at an OS level and use direct allocated NIO buffers. I don't think there's a very high risk, unless you are using a lot of native code.

          1. What if you lock the memory using native code, perform sensitive operations and never allow Java to see what the values are, except for the results of the comparison etc. ? (sensitive info could still be determined from the control flow/results)

            The equivalent C/C++ guidelines suggest a pretty thorough mechanism but I get the feeling that we cannot do much about it internally through Java.

            1. For reference purposes, here are the C rules dealing with sensitive information. (Yes, we should merge them together into one or maybe two rules.)

              Short story, there is no portable solution, but there are implementation-specitic ones. I suspect it has not (historically) been enough of a problem to generate enough demand for a portable solution.

              MEM06-C. Ensure that sensitive data is not written out to disk
              MSC06-C. Beware of compiler optimizations
              MSC18-C. Be careful while handling sensitive data, such as passwords, in program code

  2. Something wrong about the last Compliant Solution:

    return DriverManager.getConnection(
          "jdbc:mysql://localhost/dbName",
          username.toString(), password.toString());
    for (int i = username.length - 1; i >= 0; i--) { 
      username[i] = 0;
    }
    for (int i = password.length - 1; i >= 0; i--) { 
      password[i] = 0;
    }
    

    The last two for loops will never be executed after the return statement. Should we change it to use try-finally block?

    1. True. Also, converting username and password to strings for passing them to getConnection() negates the security benefits of storing them in char arrays and erasing them afterwards. So I converted the example to use strings.

  3. It would be interesting to see the automated detection section include reference to Sonar rules

    http://docs.codehaus.org/display/SONAR/Extending+Coding+Rules

      1. Well, sorta. It's difficult to tell what's "sensitive", but we can recognize when database and LDAP credentials are hard-coded.

  4. I recently wrote a stand-alone batch app in Java.  I used Spring's framework to read an encrypted password and connect to the database.  I stored the password in an environment variable set at login for a "nameless id" (basically, an ID used only for that system) and set the file privs on the .profile file to only be read by the user (like chmod 700 or something like that).

    That is as secure as I needed to get for that application.  The only person who could get the password is a person with the password for that nameless ID.  Even then, they'd have to be smart enough to look for the encrypted password in the environment variables.  Not impossible but not a likely place for it either.  Finally, they'd have to write code to decrypt it which wouldn't be that hard either if they had access to the decrypting source in the source code repository–probably something they WOULD have if they knew the password to the nameless ID (thinking a rogue developer or something).

    Combining this technique with others described here would be pretty secure as long as you maintain tight control over who knows the faceless id.

     

    Code looks like this:

    spring config file:

    -<bean class="xxx.lobsb.util.crypt.DecryptingDataSource" id="dataSource" destroy-method="close"><property value="oracle.jdbc.driver.OracleDriver" name="driverClassName"/>
    <!--Restart IDEA after setting these OS level env vars:-->
    <property value="${DB_URL}" name="url"/><property value="${DB_USER_ID}" name="username"/><property value="${LOBSB_DB_PWD}" name="password"/>
    <!-- Set in OS env var -->
    </bean>

     

    DecryptingDataSource.java:

    package xxx.lobsb.util.crypt;

    import java.sql.Connection;
    import java.sql.SQLException;

    import org.apache.commons.dbcp.BasicDataSource;

     


    public class DecryptingDataSource extends BasicDataSource {

    public Connection getConnection() {
    Connection conn = null;
    try {
    this.setPassword(ProtectedConfigFile.decrypt(password));
    conn = getConnection(username, password);
    }catch (Exception e) {
    System.err.println("GeneralSecurityException thrown: " + e.getMessage());
    }

    return conn;
    }

    public Connection getConnection(String userID, String password) {
    // String decryptedPassword = null;
    Connection conn = null;

    try {
    if(password.endsWith("=")) {
    this.setPassword(ProtectedConfigFile.decrypt(password));
    }

    conn = super.getConnection();
    }catch (Exception e) {
    System.err.println("GeneralSecurityException thrown: " + e.getMessage());
    }

    return conn;
    }

    public boolean isWrapperFor(Class<?> iface) throws SQLException {
    throw new SQLException("Not implemented.");
    }
    public <T> T unwrap(Class<T> iface) throws SQLException {
    throw new SQLException("Not implemented.");
    }

    }

    1. Note that permission 700 is also accessible by anyone with root access.

      1. True. You are now depending on the security of your filesystem. Which may or may not be secure, but it is out of the scope of Java (smile)

        Anyway, your code does not seem to be violating this rule. It hardcodes the name of your environment variable, but I presume that is not sensitive. You might have a problem if you ever store the password as a java.lang.String, because Java makes no guarantee when that String will be destroyed, if ever. Better to store passwords as a char[].

  5. I would have thought that this was more severe than MSC02-J, as it gives you system access directly.

    Also severity on the index page for this item does not match: 49. Miscellaneous (MSC).

    1. Why does the severity of this rule have to match the severity of MSC02-J? According to Priority and Levels violating this rule has medium severity b/c it leads to an info leak. Maybe the same applies to MSC02-J...not sure there.

      Soon I'll run a script that fixes the RA numbers on the Misc. page automagically.

      1. Because the information leaked leads to escalated privileges (high).

        From the linked CWE-259 "Technical Impact: Gain privileges / assume identity"

        From the linked CWE-798 "Technical Impact: Read application data; Gain privileges / assume identity; Execute unauthorized code or commands; Other"

        From the linked Priority and Levels "3 = high (run arbitrary code, privilege escalation)"

         

         

        1. I reverted the severity as you suggest, and embellished the RA text to explain why this info disclosure vulnerability has high severity instead of medium.

  6. The stream handling in the first Compliant Solution is completely below par. No try with resources, closing streams, correct buffer handling, defined character encoding etc.

    1. The first compliant solution now performs a standards-compliant read of data from a file.