Use ObjectInputFilter for deserialization

[ritvibhatt]

Description

Adds ObjectInputFilter for deserialization in PlanSerializer, DefaultExpressionSerializer, and RelJsonSerializer by implementing a allowlist-based approach that only allows specific classes.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 67351c2)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 Security concerns Deserialization gadget chain risk: The allowlist in DeserializationFilterUtil includes java.lang.invoke.SerializedLambda (line 43), which is a known component in Java deserialization gadget chains. When combined with other allowed classes like collections and Guava classes, this could potentially enable remote code execution attacks. Additionally, the wildcard patterns like org.opensearch.sql.utils.* and com.google.common.collect.** may inadvertently allow dangerous classes. Review whether SerializedLambda is necessary, and consider replacing wildcard patterns with explicit class lists to minimize the attack surface.

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Overly Permissive Filter The base allowlist includes java.lang.invoke.SerializedLambda which can be exploited for gadget chain attacks during deserialization. SerializedLambda allows arbitrary code execution when combined with other classes. Unless there is a specific requirement to deserialize lambda expressions, this class should be removed from the allowlist to prevent potential security vulnerabilities. + "java.lang.invoke.SerializedLambda;" Wildcard Pattern Risk The filter pattern org.opensearch.sql.utils.* allows all classes in the utils package to be deserialized. If any class in this package is vulnerable or becomes vulnerable in the future, it could be exploited. Consider explicitly listing only the required classes instead of using a wildcard, or ensure the utils package contains only safe, immutable data classes. + "org.opensearch.sql.utils.*;"));

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 67351c2
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Add null check for parameters Validate that additionalPatterns is not null before concatenation to prevent potential NullPointerException. Consider using an empty string as default if null is passed, or explicitly document that null values are not allowed. core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java [71-74] public static ObjectInputFilter createFilter(String additionalPatterns) { + if (additionalPatterns == null) { + additionalPatterns = ""; + } String fullPattern = BASE_ALLOWLIST + additionalPatterns + "!*"; return createLoggingFilter(ObjectInputFilter.Config.createFilter(fullPattern)); } Suggestion importance[1-10]: 5 __ Why: While adding a null check improves robustness, the impact is moderate. All current usages in the PR pass non-null values (either empty string or specific patterns), making this a defensive programming improvement rather than fixing an actual bug.	Low

---

Previous suggestions

Suggestions up to commit d5754eb

Category	Suggestion	Impact
General	Add null check for parameters Validate that additionalPatterns is not null before concatenation to prevent potential NullPointerException. Add a null check or use Objects.requireNonNull() to ensure the parameter is valid before building the filter pattern. core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java [71-74] public static ObjectInputFilter createFilter(String additionalPatterns) { + if (additionalPatterns == null) { + additionalPatterns = ""; + } String fullPattern = BASE_ALLOWLIST + additionalPatterns + "!*"; return createLoggingFilter(ObjectInputFilter.Config.createFilter(fullPattern)); } Suggestion importance[1-10]: 5 __ Why: While adding a null check for additionalPatterns improves robustness, the PR already passes empty strings ("") in all call sites, making this a minor defensive programming improvement rather than fixing an actual bug.	Low

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 67351c2.

Path	Line	Severity	Description
core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java	47	low	java.lang.invoke.SerializedLambda is included in the BASE_ALLOWLIST. This class is a known component in Java deserialization gadget chains. While lambda serialization may be legitimately needed, its inclusion reduces the effectiveness of the allowlist as a security control.
core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java	50	low	com.google.common.collect.** uses a double-wildcard that matches all Guava collection classes across all sub-packages. Guava collections have known gadget chains. The scope could be narrowed to specific classes actually needed for deserialization.
core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java	45	low	java.io.Serializable is included in the BASE_ALLOWLIST. This is a marker interface, not a concrete class, and its presence in an ObjectInputFilter allowlist has no practical effect. The entry is superfluous and mildly confusing, but harmless.

The table above displays the top 10 most important findings.

Total: 3 | Critical: 0 | High: 0 | Medium: 0 | Low: 3

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[github-actions]

Persistent review updated to latest commit 67351c2