This section helps to understand the supported code checks for Java language. Embold code checks can be enabled and disabled according to user requirement.
Note: Currently, we support Embold Java Issues only; and does not support PMD issues and SpotBugs issues.
Critical Level Issues
Rule | Description | Example | KPI | Tags |
---|---|---|---|---|
Compile Regex Once | Pattern object compiles the regular expressions which are passed to them and this compilation happens in the memory. If a regular expression is used many times, then this compilation should be performed only once. | Non-compliant code:Class Foo { Compliant code: Class Foo { | Efficiency | |
Disabled Spring Securitys CSRF | CSRF protection is enabled by default in the Java configuration. Disabling CSRF protection can create a major security threat. Cross-Site Request Forgery (CSRF) is an attack that forces an end user to execute unwanted actions on a web application in which they're currently authenticated. More Info - OWASP Top 10 A6:2017 - Security Misconfiguration CWE-352 - Cross-Site Request Forgery (CSRF) | Non-compliant code:@Configuration Compliant code: @Configuration | Security | |
Resource Leak | Resource should be closed in finally block. Another way is to use try-with-resource. In case of either exception or no exception, close() should always be put in finally clause. | Non-compliant code:class Demo { Compliant code: class Demo { | Resource Utilization | |
Weak Cipher Algorithm | Use strong cryptographic algorithms as they are less vulnerable to brute force attacks among others. For more information : : OWASP Top 10 2017 Category A3 - Sensitive Data Exposure MITRE, CWE-327 - Use of a Broken or Risky Cryptographic Algorithm | Non-compliant code: class CipherExample Compliant code:
| Security | |
Possible Thread Leak In Executor Service | Thread leak can be possible if ExecutorService is not getting shutdown.There is a queue of tasks between client threads and thread pool. When the normal thread finishes the "run" method (Runnable or Callable), it will be passed to garbage collector for collection. But with ExecuterService, threads will be simply put on hold and they will not be selected for garbage collection. Therefore, the shutdown is needed for ExecutorService. | Non-compliant code: class Demo { Compliant code: class Demo { | Resource Utilization | |
Non Private Field In Synchronized Block | Non-private, non-final field accessed in synchronized block indicates possibly partial synchronization, and does not protect direct access to the field from other parts of the system without synchronization. Make the field private and/or final, and provide accessors which can enforce synchronization. More Info - CWE-820 - Missing synchronization | Non-compliant code:class NonPrivateFieldAccessInSynchronizedBlock { Compliant code class NonPrivateFieldAccessInSynchronizedBlock { | Robustness, Security | |
Database Should Be Password Protected | Database connection should always be password protected. Attacker can access the sensitive data from the database, in case it is not secured by password. More Info - [OWASP Top 10 2017 Category A3] - Sensitive Data Exposure - [MITRE, CWE-521] - Weak Password Requirements | Non-compliant code:class CipherExample { Compliant code class CipherExample { | Security | |
Should Not Use getRequestedSessionId API | This method returns the session ID specified by the client and may not be the same as the ID of the current valid session for this request. If the client does not mention a session ID, it returns null. The session ID returned is either transmitted in a cookie or a URL parameter, hence by definition nothing prevents the end-user from manually updating the value of this session ID in the HTTP request. More Info - [CWE-807] - Reliance on Untrusted Inputs in a Security Decision - [OWASP] - Broken Authentication |
| Security | |
Web Application Contains Main Method | This method returns the session ID specified by the client and may not be the same as the ID of the current valid session for this request. If the client does not mention a session ID, it returns null. The session ID returned is either transmitted in a cookie or a URL parameter, hence by definition nothing prevents the end-user from manually updating the value of this session ID in the HTTP request. More Info - [CWE-489] - Leftover Debug Code - [OWASP] - Sensitive Data Exposure |
``` | Security | |
Authenticate LDAP Connection | Anonymous binds and unauthenticated binds allow access to information in the LDAP directory without providing a password. Therefore using these binds is strongly discouraged. More Info - [OWASP Top 10 2017 Category A2]- Broken Authentication - [CWE-521] - Weak Password Requirements - [ldapwiki.com] - Simple Authentication |
| Security | |
Potential Command Injection | Avoid using unfiltered input to process executor APIs because that can lead to arbitrary command execution. More Info - [OWASP] - Command Injection - [OWASP: Top 10 2013 A1] -Injection - [CWE-78] - Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') |
| Security | |
Potential Path Traversal | Path traversal is a web security vulnerability that allows an attacker to read arbitrary files on the server that is running an application. This might include application code and data, credentials for back-end systems, and sensitive operating system files. More Info - [CWE-22] - Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') - [OWASP] - Path Traversal |
| Security | |
Accessing Android external storage is security-sensitive | Files or data stored in the android application should be security-sensitive. Files can be globally readable or writable as they are stored on external storage. Avoid storing sensitive information on the external storage as anyone can tamper with the data or remove the files by using any application. **More Info** - [OWASP Top 10 2017 Category A1](https://owasp.org/www-project-top-ten/2017/A1_2017-Injection.html) - Injection - [OWASP Top 10 2017 Category A3](https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure) - Sensitive Data Exposure - [CWE-312](https://cwe.mitre.org/data/definitions/312.html) - Cleartext Storage of Sensitive Information - [CWE-20](https://cwe.mitre.org/data/definitions/20.html) - Improper Input Validation accessing android external storage |
| Security | |
Using unsafe Jackson deserialization configuration is security-sensitive | JThe untrusted data could cause abuse to application logic. Jackson's deserialization should be configured securely. Implement @JsonTypeName and @JsonSubTypes to prevent serialized data from an untrusted source. It can help to safeguard the data from external attackers. **More Info** - [OWASP Top 10 2017 Category A8](https://owasp.org/www-project-top-ten/2017/A8_2017-Insecure_Deserialization) - Insecure Deserialization - [CWE-502](https://cwe.mitre.org/data/definitions/502.html) - Deserialization of Untrusted Data |
| Security | |
Setting JavaBean properties is security-sensitive | The JavaBean property uses a set or get functions that are exposed to other applications. An attacker can modify its properties, attack malicious code that can be risky. Avoid storing sensitive information under this JavaBean property as it may help the user to retain its software integrity. **More Info** - [OWASP Top 10 2017 Category A8](https://owasp.org/www-project-top-ten/2017/A8_2017-Insecure_Deserialization) - Insecure Deserialization - [CWE-502](https://cwe.mitre.org/data/definitions/502.html) - Deserialization of Untrusted Data |
| Security | |
Run Should Not Be Called Directly | The program accidentally calls the Thread.run() method and causes the code to be executed in the current thread, just like any other method call. Instead, use Thread.start() to actually create a new thread so that the runnable's 'run()' method is executed in parallel. More Info [CWE-572] - Call to Thread run() instead of start() | Non-compliant code class Demo { Compliant code class Demo { | Functionality, Efficiency | |
Exclude SpringBootApplication And ComponentScan From The Default Package | Exclude "SpringBootApplication" and "@ComponentScan" from the default package. | Non-compliant code import org.springframework.boot.SpringApplication; Compliant code package javacodechecker; | Efficiency | spring |
High-Level Issues
Rule | Description | Example | KPI |
---|---|---|---|
Initialization Of Secure Random At Method Level | "SecureRandom" should not be initialized in method. Every SecureRandom generation is seeded from some entropy pool. Creating a new SecureRandom method on every call might slow down the application as it might block the creation of seed. Use a statically created instance instead. | Non-compliant code class Demo { Compliant code class SecureRandomGenerator { | Security |
Security Sensitive Regex | Regular expressions are security-sensitive. Evaluating regular expressions with input string can be an extremely CPU-intensive task. The regular expression naive algorithm builds a Nondeterministic Finite Automaton (NFA), the attacker might use the knowledge of states of finite state machine to look for applications that use regular expressions, containing an Evil Regex, and send a well-crafted input, that will hang the system. Alternatively, if a Regex itself is affected by a user input, the attacker can inject an Evil Regex, and make the system vulnerable. | Examples of Evil Patterns: Examples of Evil pattern : 1) (a+)+ 2) ([a-zA-Z]+)* 3) (a|aa)+ 4) (a|a?)+ 5) (.*a){2} for x > 10 E.g Pattern: /(a+)+b/; Input string:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab Instead use Pattern:(/a+b/) //fixed the hard-coded regex pattern to avoid possible captures, possessive quantifiers and back-references To avoid possible attacks, if the regex pattern is defined with an user-controlled input, it should be sanitized in order to escape characters which are part of the regular expression syntax. Non-compliant code class Demo { public boolean validate1(javax.servlet.http.HttpServletRequest request) { String regex = " /(a+)+b/"; String input = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab"; input.matches(regex); //Noncompliant Compliant code class Demo { public boolean | Efficiency, Security |
Use Of System.err.println | 1. System.err.println is an IO-operation and therefore is time consuming. 2. The better approach is to use a logging framework for a message queue. 3. Moreover, you can configure separate log files for different purposes. | Non-compliant code class Demo { Compliant code class Demo { | Analyzability |
Invalid Logging Class Name | Avoid incorrect class names while creating logger. Ignoring this may create confusion while analysing logs. | Non-compliant code public class LoggingClass extends BaseChecker { Compliant code public class LoggingClass extends BaseChecker { | Analyzability |
Use Of System.out.println | 1.Sending messages to stdout is usually inappropriate in a production environment. E.g. If you are coding a GUI app, the information should be presented to the user and not to the stdout method anywhere. 2.System.out.println is an IO-operation and therefore is time consuming. The better approach is to use a logging framework for a message queue. 3.Moreover, you can configure separate log files for different purposes. | Non-compliant code class Demo { Compliant code public void process2() { | Analyzability |
Complex Regex Pattern | Avoid usage of complex Regex pattern as it involves heavy processing.In some cases complex Regex performance test shows that regex is significantly inefficient. | //Non-compliant code class Demo { | Efficiency |
Empty Catch Block | Empty Catch Block finds instances where an exception is caught, but nothing is done. In most circumstances, this ignores an exception which should either be acted on or reported. Avoid keeping the catch block empty. | // Non-compliant code class Demo { // compliant code public void process2() { | Analyzability |
Empty Catch Block | Empty Catch Block finds instances where an exception is caught, but nothing is done. In most circumstances, this ignores an exception which should either be acted on or reported. Avoid keeping the catch block empty. | // Non-compliant code class Demo { // compliant code public void process2() { | Analyzability |
Sensitive Info Logged | Logger is used for recording application activity. Information displayed on the logs is visible to different stakeholders. Avoid logging sensitive information such as password, key, social security number etc. Not complying may compromise the system security. | Non-compliant code:class Demo { Compliant code class Demo { | Security |
Main Should Not Throw Anything | The main method should not throw any checked exception, instead, it needs to handle properly. If a non-checked exception is thrown (and not catch) in the main method, it will terminate. | Non-compliant code: Compliant code public static void main(String[] args) { | Functionality |
Avoid Synchronized At Method Level | Avoid synchronized at method level. When new code is added to the existing method then method-level synchronization can cause problems so to avoid these kind of problems use block-level synchronization. Block-level synchronization helps to ensure that only the code that needs synchronization gets it. | Non-compliant code:
Compliant code public class SynchronizedCounter { | Efficiency |
Shortcircuit Logic Should Be Used In Boolean Contexts | Using non-short-curcuit logic is a mistake it can also cause serious problem error. | Non-compliant code:
Compliant code
| Efficiency |
Medium Level Issues
Rule | Description | Example | KPI |
---|---|---|---|
Return Empty Array Or Collection Instead Of Null | Returning null value instead of an empty array or collection can lead to denial-of-service vulnerabilities when the client code fails to explicitly handle the null return value. For methods that return a set of values using an array or collection, returning an empty array or collection is an excellent alternative to returning a null value. More Info MET55-J - Return an empty array or collection instead of a null value for methods that return an array or collection MSC19-C - For functions that return an array, prefer returning an empty array over a null value | Non-compliant code: class Demo { Compliant code: class Demo { | Robustness |
Preserve Stack Trace In Logs | Preserve stack trace in logs. This will help to analyse the logs in case of any exception. | Non-compliant code:class Demo { Compliant code: class Demo { | Analyzability |
Read Only Transaction | Spring components support database transactions using "@Transactional" annotation. If readOnly attribute is not explicitly set to true, we will have read/write transactions for select queries. Hence, it is always recommended to explicitly specify the readOnly attribute. | Non-Compliant Code:class Demo { Compliant Code: class Demo { | Robustness |
Unusual REST Practice | The best practices while creating REST API's are : 1. URL should contain resources (E.g. nouns) only; not actions or verbs. 2. Singular and plural noun should not be mixed together. 3. Use plural noun only for all the resources. 4. Use GET method, instead of the POST method to fetch the data. 5. Use PUT, POST and DELETE methods to alter the state. | Non-compliant code class Demo { Compliant code class Demo { | Maintainability |
Variables Should Not Be Self Assigned | Self-assignment of the variables can be confusing and leads to bugs; however, one should not assign a variable to itself. Hence, this statement can be redundant and removed. | Non-compliant code class Demo { Compliant code class Demo { | Functionality |
Externalizable Must Have No Arguments Constructor | Externalizable interface cannot be deserialized without a non-argument constructor,so non-argument constructor must be implemented. | Non-compliant code public class Car implements Externalizable { Compliant code public class Car implements Externalizable { | Efficiency |
Getters And Setters Should Access The Expected Fields | Getter and Setter methods must access the expected fields. For each instance variable, a getter method returns its value, while a setter method sets or updates its value. For example, 'active' is an instance variable and 'setActive' (boolean value) is a setter method. Instead of unexpectedly updating any field, it must update or set the active variable. | Non-compliant code class Demo { Compliant code private boolean active; | Functionality |
RunFinalizersOnExit Should Not Be Called | Remove Runtime::runFinalizersOnExit and System::runFinalizersOnExit methods. It can be enabled with "System.runFinalizersOnExit" and "Runtime.runFinalizersOnExit".It may result in finalizers being called on live objects while other threads are concurrently manipulating those objects, resulting in unexpected behavior or deadlock. - [CWE-586]- Explicit Call to Finalize() | Non-compliant code public static void main(String [] args) { Compliant code public static void main(String [] args) { | Efficiency |
Big Integer Instantiation | Use already existing BigIntegers (BigInteger.ZERO, BigInteger.ONE, BigInteger.TEN).Instead of creating a new object with new BigInteger better use one static object which is created once when the BigInteger class is loaded. It is likely to yield significantly better space and time performance. Zero and One is probably the most fundamental number in mathematics. Using static objects avoids the allocation about 48 bytes and the need to collect them back later in a tight loop that can matter. | Non-compliant code BigInteger bigInteger = new BigInteger("1"); Compliant code BigInteger bigInteger = BigInteger.ONE; | Efficiency |
Maps With Enum Values Replace With EnumMap | If Map has all the key values from the same Enum then Map should be replaced with EnumMap because the underlying data structure is a simple array so it will be more efficient than other sets. | Non-compliant code public class MyClass { Compliant code public class MyClass { | Functionality |
Mismatch Regex Boundaries Should Not Be Used | In the regular expression by switching $ and ^ boundaries it will never match and it can be misused. | Noncompliant Code pattern.compile("$[a-z]+^"); Compliant Code pattern.compile("^[a-z]+$"); | Functionality |
Avoid Concatenating Char As String | Avoid concatenating characters as strings because using string rather than char creates unnecessarily space accommodation in heap space. Appending a character as a char will always be faster than appending it as a String. | Noncompliant Code class Demo { Compliant Code class Demo { | Functionality |
Empty String Should Not Be Used | Concatenating empty string with literals during conversion is inefficient. | Noncompliant Code String s = "" + 456; Compliant Code String t = Integer.toString(456); | Functionality |
Exceptions Should Not Be Thrown In Finally Block | Exception that is thrown in finally block will mask any previous exception in try or catch block and the stack trace and exception message will be lost. | Noncompliant Code try { Compliant Code try { | Functionality |
Low-Level Issues
Rule | Description | Example | KPI |
---|---|---|---|
Log Level Info In Catch Block | The catch block handles exception that occurs in associated try block. In this block, the logger level is expected to be as "error". This is a good list describing the log levels : 1. Debug : Used for development and testing. 2. Information : Used to output information that is useful to run and manage your system. 3. Warning : Used for handled 'exceptions' or other important log events. 4. Error : Used to log all unhandled exceptions. This is typically logged inside a catch block at the boundary of your application. 5. Fatal : Used for special exceptions/conditions where it is imperative that we can quickly pick out these events. | Non-compliant code: class Demo { Compliant code: class Demo { | Analyzability |
Non Thread Safe Field Declaration | A field declared inside the spring component should be thread safe. By default, beans in spring are singleton. Multiple threads accessing the same component may produce inconsistent results, if they access fields declared globally. | Non-compliant code: class Demo { Compliant code: class Demo { | Robustness |
Info Level Issues
Rule | Description | Example | KPI |
---|---|---|---|
String Concatenation In Logging | Instead of string concatenation use parameterized logging. Concatenation is calculated before the condition check. If you call your logging framework multiple times which turns to be false, this effort will be a waste of time. E.g. if you call your logging framework 10K times conditionally and all of them evaluates to be false, concatenation will happen 10K times. | Non-compliant code: class Demo { Compliant code: class Demo { | Efficiency |
Tightly Coupled Class | A spring component such as repository, service and controller should auto-wire the interface, instead of its implementation class. Ignoring this, will increase tight coupling among the classes | Non-compliant code:class Demo { Compliant code: class Demo { | Maintainability |