DeepCode’s Top Findings#4: JavaScript Attribute Access on NULL
Tags: deepcode, javascript, staticcodeanalysis, softwarebug
Published: 2020-01-08 16:19:23

[DeepCode](https://deepcode.ai) 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](https://www.deepcode.ai/app/gh/facebook/react/d7feeb25ac8cc23133c294fa01932658023130c0/_/dashboard/).

##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](https://www.researchgate.net/publication/276082849_Pointer_Analysis) of a complex object. Give it a try on your own code at [deepcode.ai](https://deepcode.ai)

CU

0xff