Skip to content

Introduced protections against deserialization attacks #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Introduced protections against deserialization attacks #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
<sonar.projectKey>iluwatar_java-design-patterns</sonar.projectKey>
<sonar.moduleKey>${project.artifactId}</sonar.moduleKey>
<sonar.projectName>Java Design Patterns</sonar.projectName>
<versions.java-security-toolkit>1.2.0</versions.java-security-toolkit>
</properties>
<modules>
<module>abstract-factory</module>
Expand Down Expand Up @@ -248,6 +249,12 @@
<version>${system-lambda.version}</version>
<scope>test</scope>
</dependency>
<dependency>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This library holds security tools for protecting Java API calls.

License: MIT ✅ | Open source ✅ | More facts

<groupId>io.github.pixee</groupId>
<artifactId>java-security-toolkit</artifactId>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This library holds security tools for protecting Java API calls.

License: MIT ✅ | Open source ✅ | More facts

<version>${versions.java-security-toolkit}</version>
</dependency>
</dependencies>
</dependencyManagement>
<dependencies>
Expand Down
4 changes: 4 additions & 0 deletions serialized-entity/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@
<groupId>com.h2database</groupId>
<artifactId>h2</artifactId>
</dependency>
<dependency>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This library holds security tools for protecting Java API calls.

License: MIT ✅ | Open source ✅ | More facts

<groupId>io.github.pixee</groupId>
<artifactId>java-security-toolkit</artifactId>
</dependency>
</dependencies>
<build>
<plugins>
Expand Down
2 changes: 2 additions & 0 deletions serialized-entity/src/main/java/com/iluwatar/serializedentity/CountrySchemaSql.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
*/
package com.iluwatar.serializedentity;

import io.github.pixee.security.ObjectInputFilters;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
Expand Down Expand Up @@ -109,6 +110,7 @@ public int selectCountry() throws IOException, ClassNotFoundException {
Blob countryBlob = rs.getBlob("country");
ByteArrayInputStream baos = new ByteArrayInputStream(countryBlob.getBytes(1, (int) countryBlob.length()));
ObjectInputStream ois = new ObjectInputStream(baos);
Comment on lines 111 to 112
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method countryBlob.getBytes(1, (int) countryBlob.length()) can throw a SQLException if the blob size exceeds the maximum size that can be converted to a byte array. Consider adding error handling for this scenario to prevent a potential denial of service if large blobs are processed.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Micro-Learning Topic: Denial of service (Detected by phrase)

Matched on "denial of service"

The Denial of Service (DoS) attack is focused on making a resource (site, application, server) unavailable for the purpose it was designed. There are many ways to make a service unavailable for legitimate users by manipulating network packets, programming, logical, or resources handling vulnerabilities, among others. Source: https://www.owasp.org/index.php/Denial_of_Service

Try a challenge in Secure Code Warrior

ObjectInputFilters.enableObjectFilterIfUnprotected(ois);
country = (Country) ois.readObject();
LOGGER.info("Country: " + country);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging the entire Country object at INFO level could potentially expose sensitive data in the logs. Consider logging only non-sensitive information or increasing the log level to DEBUG to reduce the visibility of potentially sensitive data.

}
Expand Down
2 changes: 2 additions & 0 deletions serialized-entity/src/test/java/com/iluwatar/serializedentity/CountryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
* THE SOFTWARE.
*/
package com.iluwatar.serializedentity;
import io.github.pixee.security.ObjectInputFilters;
import org.junit.jupiter.api.Test;

import java.io.*;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a wildcard import for java.io.* can lead to namespace pollution and ambiguity in large projects. It's generally better to import only the specific classes needed.

Recommended Solution:
Replace the wildcard import with specific class imports, e.g., import java.io.FileInputStream; and import java.io.ObjectInputStream;.

Expand Down Expand Up @@ -85,6 +86,7 @@ void testSerializable(){
// De-serialize Country
try {
ObjectInputStream objectInputStream = new ObjectInputStream(new FileInputStream("output.txt"));
ObjectInputFilters.enableObjectFilterIfUnprotected(objectInputStream);
Country country = (Country) objectInputStream.readObject();
objectInputStream.close();
System.out.println(country);
Comment on lines 91 to 92
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using System.out.println in test methods is not recommended as it does not contribute to test assertions and can clutter the test output. Additionally, catching a generic Exception is too broad and can mask other unexpected issues.

Recommended Solution:

  • Remove the System.out.println statement.
  • Replace the generic Exception catch with more specific exceptions, such as IOException and ClassNotFoundException, to handle expected issues more precisely.

Expand Down
4 changes: 4 additions & 0 deletions tolerant-reader/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@
<artifactId>junit-jupiter-engine</artifactId>
<scope>test</scope>
</dependency>
<dependency>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This library holds security tools for protecting Java API calls.

License: MIT ✅ | Open source ✅ | More facts

<groupId>io.github.pixee</groupId>
<artifactId>java-security-toolkit</artifactId>
</dependency>
</dependencies>
<build>
<plugins>
Expand Down
2 changes: 2 additions & 0 deletions tolerant-reader/src/main/java/com/iluwatar/tolerantreader/RainbowFishSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
*/
package com.iluwatar.tolerantreader;

import io.github.pixee.security.ObjectInputFilters;
import java.io.FileInputStream;
import java.io.FileOutputStream;
Comment on lines 28 to 29
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The direct use of FileInputStream and FileOutputStream within the class methods can lead to tight coupling with the file system, which may hinder unit testing and flexibility in data handling. Consider abstracting file operations behind an interface to improve modularity and testability. For instance:

interface DataStreamFactory {
    InputStream createInputStream(String path) throws IOException;
    OutputStream createOutputStream(String path) throws IOException;
}

This approach allows for easier mocking during testing and can accommodate different storage mechanisms without modifying the core serialization logic.

import java.io.IOException;
Expand Down Expand Up @@ -90,6 +91,7 @@ public static RainbowFish readV1(String filename) throws IOException, ClassNotFo

try (var fileIn = new FileInputStream(filename);
var objIn = new ObjectInputStream(fileIn)) {
ObjectInputFilters.enableObjectFilterIfUnprotected(objIn);
map = (Map<String, String>) objIn.readObject();
Comment on lines +94 to 95
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of ObjectInputFilters.enableObjectFilterIfUnprotected(objIn); is crucial for security to prevent deserialization vulnerabilities. However, ensure that this method effectively blocks deserialization of potentially harmful classes. It's recommended to explicitly define which classes are allowed or disallowed to further tighten security. For example:

ObjectInputFilter filter = ObjectInputFilter.Config.createFilter("com.iluwatar.*;java.base/*;!");
objIn.setObjectInputFilter(filter);

This configuration explicitly allows certain packages while blocking others, reducing the risk of unwanted or malicious object creation during deserialization.

}

Expand Down
Loading