DeepCode’s Top Findings#4: JavaScript Attribute Access on NULL

DeepCode offers an AI-based Static Program Analysis for Java, Javascript and Typescript, and Python. You might know, DeepCode uses thousands of open source repos to train our engine. We asked the engine team to provide some stats on the findings. For the top suggestions from our engine, we want to provide an introduction and give some background in this series of blog articles. This is #4 in this series...

Language: JavaScript
Defect: Attribute Access On Null (Category General 3)
Diagnose: – Null reference via read access / Conditional execution context determines an accessed variable to hold null.

You can find below example by in Facebook’s React here.

Background

Let us have a look at the code snippet of the aforementioned example (sorry I know it is a bit lengthy) and follow the variable maybeInstance:

...
/**
    * This function sends props straight to native. They will not participate in
    * future diff process - this means that if you do not include them in the
    * next render, they will remain active (see [Direct
    * Manipulation](docs/direct-manipulation.html)).
    */
setNativeProps: function(nativeProps: Object) {
    // Class components don't have viewConfig -> validateAttributes.
    // Nor does it make sense to set native props on a non-native component.
    // Instead, find the nearest host component and set props on it.
    // Use findNodeHandle() rather than findNodeHandle() because
    // We want the instance/wrapper (not the native tag).
    let maybeInstance;

    // Fiber errors if findNodeHandle is called for an umounted component.
    // Tests using ReactTestRenderer will trigger this case indirectly.
    // Mimicking stack behavior, we should silently ignore this case.
    // TODO Fix ReactTestRenderer so we can remove this try/catch.
    try {
        maybeInstance = findHostInstance(this); 
    } catch (error) {}

    // If there is no host component beneath this we should fail silently.
    // This is not an error; it could mean a class component rendered null.
    if (maybeInstance == null) {
        return;
    }

    if (maybeInstance.canonical) {
        warningWithoutStack(
            false,
            'Warning: setNativeProps is not currently supported in Fabric',
        );
        return;
    }

    const nativeTag =
    maybeInstance._nativeTag || maybeInstance.canonical._nativeTag;
    const viewConfig: ReactNativeBaseComponentViewConfig<> =
    maybeInstance.viewConfig || maybeInstance.canonical.viewConfig;
...

If you look at the code, you see the variable is assigned to the result of a function ( findHostInstance() ) and exceptions are caught but not handled. Well, as the comment says: TODO.

Next, we have an if-statement which will execute the return if our variable is either null or unknown. We could argue about the == versus the === and handling the return in the catch but we leave this for another day. So, from now on we know maybeInstance is not null nor unknown.

The next if-statement tests if there is a property called canonical in maybeInstance that exists and is not null, unknown, false, or 0 as these will trigger this if and we will return from this function. So, now we know that maybeInstance.canonical is a falsy literal.

Finally, the code checks whether there is a property maybeInstance._nativeTag or a property maybeInstance.canonical._nativeTag. What happens now? JavaScript will try to interpret both sides of the OR. We know maybeInstance.canonical is either false or 0 and the program tests if these values have a property called _nativeTag. In this case, the program does not crash but this is for sure also not intended. It is very likely maybeInstance.canonical will result in null or unknown which will crash the program (Uncaught TypeError). And the line next does the same thing again...

As you saw this is not an easy thing to detect for a Static Program Analysis as you have to do a pointer analysis of a complex object. Give it a try on your own code at deepcode.ai

CU

0xff