runtime: Add null pointer exception for read (#2780)

* runtime: Throw null pointer exception on read

* runtime: Add test for null pointer exception on operator overloading

* runtime: Update README.md AS version

* runtime: Add arity to invoke_export test function name

* runtime: Make size_of_rt_size a constant

* runtime: Validate pointer access instead of using coersion on tests

graph/src/runtime/asc_ptr.rs

@@ -6,6 +6,9 @@ use std::fmt;
 use std::marker::PhantomData;
 use std::mem::size_of;
 
+/// The `rt_size` field contained in an AssemblyScript header has a size of 4 bytes.
+const SIZE_OF_RT_SIZE: u32 = 4;
+
 /// A pointer to an object in the Asc heap.
 pub struct AscPtr<C>(u32, PhantomData<C>);
 
@@ -150,12 +153,12 @@ impl<C: AscType> AscPtr<C> {
     /// This function returns the `rt_size`.
     /// Only used for version >= 0.0.5.
     pub fn read_len<H: AscHeap + ?Sized>(&self, heap: &H) -> Result<u32, DeterministicHostError> {
-        let size_of_rt_size = 4;
-        let start_of_rt_size = self.0.checked_sub(size_of_rt_size).ok_or_else(|| {
-            DeterministicHostError(anyhow::anyhow!(
-                "Subtract overflow on pointer: {}", // Usually when pointer is zero because of null in AssemblyScript
-                self.0
-            ))
+        // We're trying to read the pointer below, we should check it's
+        // not null before using it.
+        self.check_is_not_null()?;
+
+        let start_of_rt_size = self.0.checked_sub(SIZE_OF_RT_SIZE).ok_or_else(|| {
+            DeterministicHostError(anyhow::anyhow!("Subtract overflow on pointer: {}", self.0))
         })?;
         let raw_bytes = heap.get(start_of_rt_size, size_of::<u32>() as u32)?;
         let mut u32_bytes: [u8; size_of::<u32>()] = [0; size_of::<u32>()];
@@ -173,6 +176,21 @@ impl<C: AscType> AscPtr<C> {
         self.0 == 0
     }
 
+    /// There's no problem in an AscPtr being 'null' (see above AscPtr::is_null function).
+    /// However if one tries to read that pointer, it should fail with a helpful error message,
+    /// this function does this error handling.
+    ///
+    /// Summary: ALWAYS call this before reading an AscPtr.
+    pub fn check_is_not_null(&self) -> Result<(), DeterministicHostError> {
+        if self.is_null() {
+            return Err(DeterministicHostError(anyhow::anyhow!(
+                "Tried to read AssemblyScript value that is 'null'. Suggestion: look into the function that the error happened and add 'log' calls till you find where a 'null' value is being used as non-nullable. It's likely that you're calling a 'graph-ts' function (or operator) with a 'null' value when it doesn't support it."
+            )));
+        }
+
+        Ok(())
+    }
+
     // Erase type information.
     pub fn erase(self) -> AscPtr<()> {
         AscPtr::new(self.0)

runtime/test/README.md

@@ -5,7 +5,7 @@ These are the unit tests that check if the WASM runtime code is working. For now
 We support two versions of their compiler/language for now:
 
 - [`v0.6`](https://github.com/AssemblyScript/assemblyscript/releases/tag/v0.6)
-- +[`v0.19.2`](https://github.com/AssemblyScript/assemblyscript/releases/tag/v0.19.2)
+- +[`v0.19.10`](https://github.com/AssemblyScript/assemblyscript/releases/tag/v0.19.10)
 
 Because the internal ABIs changed between these versions, the runtime was added, etc, we had to duplicate the source files used for the tests (`.ts` and `.wasm`).
 
@@ -14,7 +14,7 @@ If you look into the [`wasm_test`](https://github.com/graphprotocol/graph-node/t
 - [`api_version_0_0_4`](https://github.com/graphprotocol/graph-node/tree/master/runtime/test/wasm_test/api_version_0_0_4)
 - [`api_version_0_0_5`](https://github.com/graphprotocol/graph-node/tree/master/runtime/test/wasm_test/api_version_0_0_5)
 
-This is because the first one (`0.0.4` `apiVersion`) is related to the `v0.6` of `AssemblyScript` and the second (`0.0.5` `apiVersion`) to +`v0.19.2`.
+This is because the first one (`0.0.4` `apiVersion`) is related to the `v0.6` of `AssemblyScript` and the second (`0.0.5` `apiVersion`) to +`v0.19.10`.
 
 ## How to change the `.ts`/`.wasm` files
 
@@ -36,13 +36,13 @@ asc wasm_test/api_version_0_0_4/abi_classes.ts -b wasm_test/api_version_0_0_4/ab
 
 ### Api Version 0.0.5
 
-First make sure your `asc` version is +`v0.19.2`, to check use `asc --version`.
+First make sure your `asc` version is +`v0.19.10`, to check use `asc --version`.
 
 To install the correct one use:
 
 ```
 # for the precise one
-npm install -g assemblyscript@0.19.2
+npm install -g assemblyscript@0.19.10
 
 # for the latest one, it should work as well
 npm install -g assemblyscript

runtime/test/src/test.rs

@@ -119,8 +119,9 @@ fn test_module(
 }
 
 trait WasmInstanceExt {
-    fn invoke_export0(&self, f: &str);
-    fn invoke_export<C, R>(&self, f: &str, arg: AscPtr<C>) -> AscPtr<R>;
+    fn invoke_export0_void(&self, f: &str);
+    fn invoke_export0<R>(&self, f: &str) -> AscPtr<R>;
+    fn invoke_export1<C, R>(&self, f: &str, arg: AscPtr<C>) -> AscPtr<R>;
     fn invoke_export2<C, D, R>(&self, f: &str, arg0: AscPtr<C>, arg1: AscPtr<D>) -> AscPtr<R>;
     fn invoke_export2_void<C, D>(
         &self,
@@ -133,12 +134,18 @@ trait WasmInstanceExt {
 }
 
 impl WasmInstanceExt for WasmInstance<Chain> {
-    fn invoke_export0(&self, f: &str) {
+    fn invoke_export0_void(&self, f: &str) {
         let func = self.get_func(f).typed().unwrap().clone();
         let _: () = func.call(()).unwrap();
     }
 
-    fn invoke_export<C, R>(&self, f: &str, arg: AscPtr<C>) -> AscPtr<R> {
+    fn invoke_export0<R>(&self, f: &str) -> AscPtr<R> {
+        let func = self.get_func(f).typed().unwrap().clone();
+        let ptr: u32 = func.call(()).unwrap();
+        ptr.into()
+    }
+
+    fn invoke_export1<C, R>(&self, f: &str, arg: AscPtr<C>) -> AscPtr<R> {
         let func = self.get_func(f).typed().unwrap().clone();
         let ptr: u32 = func.call(arg.wasm_ptr()).unwrap();
         ptr.into()
@@ -203,7 +210,7 @@ fn test_json_conversions(api_version: Version) {
     // test BigInt conversion
     let number = "-922337203685077092345034";
     let number_ptr = asc_new(&mut module, number).unwrap();
-    let big_int_obj: AscPtr<AscBigInt> = module.invoke_export("testToBigInt", number_ptr);
+    let big_int_obj: AscPtr<AscBigInt> = module.invoke_export1("testToBigInt", number_ptr);
     let bytes: Vec<u8> = asc_get(&module, big_int_obj).unwrap();
     assert_eq!(
         scalar::BigInt::from_str(number).unwrap(),
@@ -235,15 +242,15 @@ fn test_json_parsing(api_version: Version) {
     let s = "foo"; // Invalid because there are no quotes around `foo`
     let bytes: &[u8] = s.as_ref();
     let bytes_ptr = asc_new(&mut module, bytes).unwrap();
-    let return_value: AscPtr<AscString> = module.invoke_export("handleJsonError", bytes_ptr);
+    let return_value: AscPtr<AscString> = module.invoke_export1("handleJsonError", bytes_ptr);
     let output: String = asc_get(&module, return_value).unwrap();
     assert_eq!(output, "ERROR: true");
 
     // Parse valid JSON and get it back
     let s = "\"foo\""; // Valid because there are quotes around `foo`
     let bytes: &[u8] = s.as_ref();
     let bytes_ptr = asc_new(&mut module, bytes).unwrap();
-    let return_value: AscPtr<AscString> = module.invoke_export("handleJsonError", bytes_ptr);
+    let return_value: AscPtr<AscString> = module.invoke_export1("handleJsonError", bytes_ptr);
     let output: String = asc_get(&module, return_value).unwrap();
     assert_eq!(output, "OK: foo, ERROR: false");
 }
@@ -277,7 +284,7 @@ async fn test_ipfs_cat(api_version: Version) {
             api_version,
         );
         let arg = asc_new(&mut module, &hash).unwrap();
-        let converted: AscPtr<AscString> = module.invoke_export("ipfsCatString", arg);
+        let converted: AscPtr<AscString> = module.invoke_export1("ipfsCatString", arg);
         let data: String = asc_get(&module, converted).unwrap();
         assert_eq!(data, "42");
     })
@@ -488,7 +495,7 @@ async fn test_ipfs_fail(api_version: Version) {
 
         let hash = asc_new(&mut module, "invalid hash").unwrap();
         assert!(module
-            .invoke_export::<_, AscString>("ipfsCat", hash)
+            .invoke_export1::<_, AscString>("ipfsCat", hash)
             .is_null());
     })
     .join()
@@ -517,7 +524,7 @@ fn test_crypto_keccak256(api_version: Version) {
     let input: &[u8] = "eth".as_ref();
     let input: AscPtr<Uint8Array> = asc_new(&mut module, input).unwrap();
 
-    let hash: AscPtr<Uint8Array> = module.invoke_export("hash", input);
+    let hash: AscPtr<Uint8Array> = module.invoke_export1("hash", input);
     let hash: Vec<u8> = asc_get(&module, hash).unwrap();
     assert_eq!(
         hex::encode(hash),
@@ -548,21 +555,21 @@ fn test_big_int_to_hex(api_version: Version) {
     // Convert zero to hex
     let zero = BigInt::from_unsigned_u256(&U256::zero());
     let zero: AscPtr<AscBigInt> = asc_new(&mut module, &zero).unwrap();
-    let zero_hex_ptr: AscPtr<AscString> = module.invoke_export("big_int_to_hex", zero);
+    let zero_hex_ptr: AscPtr<AscString> = module.invoke_export1("big_int_to_hex", zero);
     let zero_hex_str: String = asc_get(&module, zero_hex_ptr).unwrap();
     assert_eq!(zero_hex_str, "0x0");
 
     // Convert 1 to hex
     let one = BigInt::from_unsigned_u256(&U256::one());
     let one: AscPtr<AscBigInt> = asc_new(&mut module, &one).unwrap();
-    let one_hex_ptr: AscPtr<AscString> = module.invoke_export("big_int_to_hex", one);
+    let one_hex_ptr: AscPtr<AscString> = module.invoke_export1("big_int_to_hex", one);
     let one_hex_str: String = asc_get(&module, one_hex_ptr).unwrap();
     assert_eq!(one_hex_str, "0x1");
 
     // Convert U256::max_value() to hex
     let u256_max = BigInt::from_unsigned_u256(&U256::max_value());
     let u256_max: AscPtr<AscBigInt> = asc_new(&mut module, &u256_max).unwrap();
-    let u256_max_hex_ptr: AscPtr<AscString> = module.invoke_export("big_int_to_hex", u256_max);
+    let u256_max_hex_ptr: AscPtr<AscString> = module.invoke_export1("big_int_to_hex", u256_max);
     let u256_max_hex_str: String = asc_get(&module, u256_max_hex_ptr).unwrap();
     assert_eq!(
         u256_max_hex_str,
@@ -696,7 +703,7 @@ fn test_bytes_to_base58(api_version: Version) {
     let bytes = hex::decode("12207D5A99F603F231D53A4F39D1521F98D2E8BB279CF29BEBFD0687DC98458E7F89")
         .unwrap();
     let bytes_ptr = asc_new(&mut module, bytes.as_slice()).unwrap();
-    let result_ptr: AscPtr<AscString> = module.invoke_export("bytes_to_base58", bytes_ptr);
+    let result_ptr: AscPtr<AscString> = module.invoke_export1("bytes_to_base58", bytes_ptr);
     let base58: String = asc_get(&module, result_ptr).unwrap();
     assert_eq!(base58, "QmWmyoMoctfbAaiEs2G46gpeUmhqFRDW6KWo64y5r581Vz");
 }
@@ -778,13 +785,13 @@ fn test_ens_name_by_hash(api_version: Version) {
     let name = "dealdrafts";
     test_store::insert_ens_name(hash, name);
     let val = asc_new(&mut module, hash).unwrap();
-    let converted: AscPtr<AscString> = module.invoke_export("nameByHash", val);
+    let converted: AscPtr<AscString> = module.invoke_export1("nameByHash", val);
     let data: String = asc_get(&module, converted).unwrap();
     assert_eq!(data, name);
 
     let hash = asc_new(&mut module, "impossible keccak hash").unwrap();
     assert!(module
-        .invoke_export::<_, AscString>("nameByHash", hash)
+        .invoke_export1::<_, AscString>("nameByHash", hash)
         .is_null());
 }
 
@@ -823,7 +830,7 @@ fn test_entity_store(api_version: Version) {
 
     let get_user = move |module: &mut WasmInstance<Chain>, id: &str| -> Option<Entity> {
         let id = asc_new(module, id).unwrap();
-        let entity_ptr: AscPtr<AscEntity> = module.invoke_export("getUser", id);
+        let entity_ptr: AscPtr<AscEntity> = module.invoke_export1("getUser", id);
         if entity_ptr.is_null() {
             None
         } else {
@@ -944,7 +951,7 @@ fn test_allocate_global(api_version: Version) {
     );
 
     // Assert globals can be allocated and don't break the heap
-    module.invoke_export0("assert_global_works");
+    module.invoke_export0_void("assert_global_works");
 }
 
 #[tokio::test]
@@ -956,3 +963,41 @@ async fn allocate_global_v0_0_5() {
     // evaluated).
     test_allocate_global(API_VERSION_0_0_5);
 }
+
+fn test_null_ptr_read(api_version: Version) {
+    let module = test_module(
+        "NullPtrRead",
+        mock_data_source(
+            &wasm_file_path("null_ptr_read.wasm", api_version.clone()),
+            api_version.clone(),
+        ),
+        api_version,
+    );
+
+    module.invoke_export0_void("nullPtrRead");
+}
+
+#[tokio::test]
+#[should_panic(expected = "Tried to read AssemblyScript value that is 'null'")]
+async fn null_ptr_read_0_0_5() {
+    test_null_ptr_read(API_VERSION_0_0_5);
+}
+
+fn test_safe_null_ptr_read(api_version: Version) {
+    let module = test_module(
+        "SafeNullPtrRead",
+        mock_data_source(
+            &wasm_file_path("null_ptr_read.wasm", api_version.clone()),
+            api_version.clone(),
+        ),
+        api_version,
+    );
+
+    module.invoke_export0_void("safeNullPtrRead");
+}
+
+#[tokio::test]
+#[should_panic(expected = "Failed to sum BigInts because left hand side is 'null'")]
+async fn safe_null_ptr_read_0_0_5() {
+    test_safe_null_ptr_read(API_VERSION_0_0_5);
+}