Skip to content

Fix NPE from Boolean auto-unboxing #3392

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arthurscchan
Copy link

@arthurscchan arthurscchan commented Mar 31, 2025

This is a proposed fix to stability issue discovered by OSS-Fuzz when fuzzing the powsybl-core module. The original OSS-Fuzz issue can be found in https://issues.oss-fuzz.com/u/1/issues/406871272 and https://issues.oss-fuzz.com/u/1/issues/406999127.

Remark This fix could also be done by defaulting variableSet to false if it is found missing from the JSON.

@arthurscchan
Copy link
Author

arthurscchan commented Mar 31, 2025

The method parseJson uses an internal ParsingContext object to accumulate parsed values from a JSON object. One of the fields in ParsingContext is declared as a boxed Boolean with default value equals to null.

static final class ParsingContext {
SensitivityFunctionType functionType;
String functionId;
SensitivityVariableType variableType;
String variableId;
Boolean variableSet;
ContingencyContextType contingencyContextType;
String contingencyId;

The problem is, the same variable variableSet in the outer SensitivityFactor is defined as primitive boolean instead of Boolean class object. This create a auto-boxing/unboxing when transferring between these two varaibles.

public class SensitivityFactor {
private final SensitivityFunctionType functionType;
private final String functionId;
private final SensitivityVariableType variableType;
private final String variableId;
private final boolean variableSet;
private final ContingencyContext contingencyContext;

Later, the code unconditionally unboxes it when calling the constructor:

} else if (token == JsonToken.END_OBJECT) {
return new SensitivityFactor(context.functionType, context.functionId, context.variableType, context.variableId, context.variableSet,
new ContingencyContext(context.contingencyId, context.contingencyContextType));
}

If the input JSON is missing the variableSet field, context.variableSet will remain null, and unboxing this will cause the NullPointerException.

@arthurscchan
Copy link
Author

This is a stability issue caused by insufficient validation during the conversion between primitive and object variables. In many cases, object wrappers for primitive types accept a wider range of values than their primitive counterparts. For example, a Boolean object can be null in addition to true or false. This becomes problematic when a null Boolean object is auto-unboxed to a primitive boolean value, as the process internally calls the booleanValue() method on the Boolean object—resulting in a NullPointerException if the object is null.

Below is a Proof of Concept (PoC) demonstrating the issue. It calls the SensitivityFactor::parseJson method with a crafted JSON string that omits the variableSet key, forcing a null unboxing scenario.

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonParser;
import com.powsybl.sensitivity.SensitivityFactor;
import java.io.StringReader;

public class ProofOfConcept {
    public static void main(String[] args) throws Exception {
        String json = """
            {
              "functionType": "BUS_VOLTAGE",
              "functionId": "branch1",
              "variableType": "BUS_TARGET_VOLTAGE",
              "variableId": "gen1",
              "contingencyContextType": "NONE"
            }
            """;

        JsonFactory factory = new JsonFactory();
        JsonParser parser = factory.createParser(new StringReader(json));
        parser.nextToken();
        SensitivityFactor.parseJson(parser);
    }
}

To execute and test the PoC, follow the steps below. It is assumed that OpenJDK 17.0.2 and Maven 3.9.9 is used.

# Prepare OpenJDK 17.0.2
wget https://download.java.net/java/GA/jdk17.0.2/dfd4a8d0985749f896bed50d7138ee7f/8/GPL/openjdk-17.0.2_linux-x64_bin.tar.gz && tar zxvf openjdk-17.0.2_linux-x64_bin.tar.gz && rm openjdk-17.0.2_linux-x64_bin.tar.gz
export JAVA_HOME=./jdk-17.0.2
export PATH=$JAVA_HOME/bin:$PATH

# Prepare Maven 3.9.9
wget https://dlcdn.apache.org/maven/maven-3/3.9.9/binaries/apache-maven-3.9.9-bin.tar.gz && tar zxvf apache-maven-3.9.9-bin.tar.gz && rm apache-maven-3.9.9-bin.tar.gz
export PATH_TO_MVN=./apache-maven-3.9.9/bin/mvn

# Build Powsybl-core
git clone https://github.com/powsybl/powsybl-core
cd powsybl-core
$PATH_TO_MVN clean package -DskipTests

# Group jar files
mkdir jar
for jar in $(find ./ -type f -name "*.jar"); do cp $jar jar/; done

# Build and run PoC
javac -cp "jar/*" ProofOfConcept.java
java -cp "jar/*:./" ProofOfConcept

You will get the following exception stack trace.

Exception in thread "main" java.lang.NullPointerException: Cannot invoke "java.lang.Boolean.booleanValue()" because "context.variableSet" is null
        at com.powsybl.sensitivity.SensitivityFactor.parseJson(SensitivityFactor.java:160)
        at ProofOfConcept.main(ProofOfConcept.java:21)

@arthurscchan
Copy link
Author

arthurscchan commented Mar 31, 2025

The issue stems from the constructor of the SensitivityFactor class during the auto-unboxing process. The appropriate fix depends on whether the absence of variableSet (i.e., a null value) is considered valid in the JSON input.

Here are two proposed fixes for different situations.

Fix 1 – Null Values Not Permitted:

  • Assumes variableSet cannot be null (i.e., it must be explicitly provided in the JSON string).
  • Modifies the SensitivityFactor constructor to accept a Boolean object instead of a primitive boolean.
  • Enforces non-null values using Objects.requireNonNull to validate the parsed JSON.

Fix 2 – Null Values Default to false:

  • Treats missing variableSet in the JSON string as a valid case, defaulting to false when null.
  • Similarly updates the constructor to accept a Boolean object.
  • Uses a ternary operator instead of a null check.
  • arthurscchan@e391469

Fix 1 is used in this PR, we can change the fix to Fix 2 if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant