fix(ffi): prevent undefined behavior on empty slices (#894)

Author: alarso16

ffi/firewood_test.go

@@ -704,3 +704,43 @@ func TestFakeRevision(t *testing.T) {
 	_, err = db.Revision(validRoot)
 	require.ErrorIs(t, err, errRevisionNotFound, "Revision(valid root)")
 }
+
+// Tests that edge case `Get` calls are handled correctly.
+func TestGetNilCases(t *testing.T) {
+	db := newTestDatabase(t)
+
+	// Commit 10 key-value pairs.
+	keys := make([][]byte, 20)
+	vals := make([][]byte, 20)
+	for i := range keys {
+		keys[i] = keyForTest(i)
+		vals[i] = valForTest(i)
+	}
+	root, err := db.Update(keys[:10], vals[:10])
+	require.NoError(t, err, "Update")
+
+	// Create the other views
+	proposal, err := db.Propose(keys[10:], vals[10:])
+	require.NoError(t, err, "Propose")
+	revision, err := db.Revision(root)
+	require.NoError(t, err, "Revision")
+
+	// Create edge case keys.
+	specialKeys := [][]byte{
+		nil,
+		{}, // empty slice
+	}
+	for _, k := range specialKeys {
+		got, err := db.Get(k)
+		require.NoError(t, err, "db.Get(%q)", k)
+		assert.Empty(t, got, "db.Get(%q)", k)
+
+		got, err = revision.Get(k)
+		require.NoError(t, err, "Revision.Get(%q)", k)
+		assert.Empty(t, got, "Revision.Get(%q)", k)
+
+		got, err = proposal.Get(k)
+		require.NoError(t, err, "Proposal.Get(%q)", k)
+		assert.Empty(t, got, "Proposal.Get(%q)", k)
+	}
+}

ffi/src/lib.rs

@@ -296,8 +296,10 @@ fn batch(
     values: *const KeyValue,
 ) -> Result<Value, String> {
     let start = coarsetime::Instant::now();
-    // Check db is valid.
     let db = unsafe { db.as_ref() }.ok_or_else(|| String::from("db should be non-null"))?;
+    if values.is_null() {
+        return Err(String::from("key-value list is null"));
+    }
 
     // Create a batch of operations to perform.
     let key_value_ref = unsafe { std::slice::from_raw_parts(values, nkeys) };
@@ -366,6 +368,9 @@ fn propose_on_db(
     values: *const KeyValue,
 ) -> Result<ProposalId, String> {
     let db = unsafe { db.as_ref() }.ok_or_else(|| String::from("db should be non-null"))?;
+    if values.is_null() {
+        return Err(String::from("key-value list is null"));
+    }
 
     // Create a batch of operations to perform.
     let key_value_ref = unsafe { std::slice::from_raw_parts(values, nkeys) };
@@ -424,6 +429,9 @@ fn propose_on_proposal(
     values: *const KeyValue,
 ) -> Result<ProposalId, String> {
     let db = unsafe { db.as_ref() }.ok_or_else(|| String::from("db should be non-null"))?;
+    if values.is_null() {
+        return Err(String::from("key-value list is null"));
+    }
 
     // Create a batch of operations to perform.
     let key_value_ref = unsafe { std::slice::from_raw_parts(values, nkeys) };
@@ -594,7 +602,11 @@ impl Display for Value {
 impl Value {
     #[must_use]
     pub const fn as_slice(&self) -> &[u8] {
-        unsafe { std::slice::from_raw_parts(self.data, self.len) }
+        if self.data.is_null() {
+            &[]
+        } else {
+            unsafe { std::slice::from_raw_parts(self.data, self.len) }
+        }
     }
 }