Repository: cossacklabs/acra (language: python)

Issue:
[ISSUE] Acra replaces null values by an empty string when using prepared statements
**Describe the bug**
When an INSERT is executed on a table using encryption, if the query explicitly sets a value to NULL, and is executed using a prepared statement, Acra replaces the NULL value by an empty string.

**To Reproduce**
Steps to reproduce the behavior:
1. Run the following java code:
```
import java.sql.*;

public class AcraTest {

    public static void main(String[] args) throws Exception {
        Class.forName("org.postgresql.Driver");
        String url = "jdbc:postgresql://localhost:9393/testdb";
        Connection conn = DriverManager.getConnection(url, "postgres", "postgres");
        final Statement statement = conn.createStatement();

        statement.execute("CREATE TABLE mytable (name bytea, age text)");

        // Execute INSERT directly
        statement.execute("INSERT INTO mytable (name, age) values ('John', NULL)");

        // Execute INSERT with a prepared statement
        PreparedStatement st = conn.prepareStatement("INSERT INTO mytable (name, age) values (?, ?)");
        st.setString(1, "Peter");
        st.setString(2, null);
        st.execute();
        st.close();

        // Check values inserted
        final ResultSet resultSet = statement.executeQuery("SELECT * FROM mytable");
        while (resultSet.next()) {
            System.out.println(String.format(
                    "Name: %s | Age: %s | Null: %s",
                    resultSet.getString("name"),
                    resultSet.getString("age"),
                    resultSet.getString("age") == null
            ));
        }
        conn.close();
    }
}

```
Output:
Name: John | Age: null | Null: true
Name: Peter | Age:  | Null: false

**Expected behavior**
Acra should have the same behavior whether the INSERT is executed on a prepared statement or not and should not replace null values.

**Acra configuration files**
For AcraServer: 
    - [ ] `encryptor_config.yaml`
```
- table: mytable
  columns:
    - name
    - age
  encrypted:
    - column: name
      data_type: str
```

**Environment:**
- Acra version: 2ff0799823d382f38cdbb3b33c82740f6df4d049
- Database server and its version: PostgreSQL 9.6.10
- Installed components:
   - [ ] AcraServer
- Data-in-transit encryption between Acra and the client-side application:
   - [ ] no transport encryption
- Installation way:
   - [ ] via Docker


**Additional context**
Add any other context about the problem here.



Discussion / hints:

Fix patch:
diff --git a/CHANGELOG_DEV.md b/CHANGELOG_DEV.md
index 7ca1786b5..d200637f1 100644
--- a/CHANGELOG_DEV.md
+++ b/CHANGELOG_DEV.md
@@ -1,3 +1,6 @@
+# 0.94.0 - 2022-10-13
+- Improve handling binary format of prepared statements for PostgreSQL with null values in Bind packets.
+
 # 0.94.0 - 2022-10-05
 - Refactored integration tests. Improved finalizing socket connections in AsyncpgExecutor. Allow to run tests with databases accessible via domain name not only localhost.
 - Added additional certificates for integration tests in buildbot runner.
diff --git a/decryptor/postgresql/utils.go b/decryptor/postgresql/utils.go
index a32ebacb0..11ff0330d 100644
--- a/decryptor/postgresql/utils.go
+++ b/decryptor/postgresql/utils.go
@@ -512,11 +512,16 @@ func writeParameterArray(buf *bytes.Buffer, parameters [][]byte) (int, error) {
 	}
 	binary.BigEndian.PutUint16(tmp[0:2], uint16(len(parameters)))
 	buf.Write(tmp[0:2])
-
 	for _, parameter := range parameters {
 		if len(parameter) > math.MaxUint32 {
 			return 0, ErrArrayTooBig
 		}
+		if parameter == nil {
+			// -1 is NULL value, 0xffffff is -1 in uint32
+			binary.BigEndian.PutUint32(tmp[0:4], math.MaxUint32)
+			buf.Write(tmp[0:4])
+			continue
+		}
 		binary.BigEndian.PutUint32(tmp[0:4], uint32(len(parameter)))
 		buf.Write(tmp[0:4])
 		buf.Write(parameter)


Test patch:
diff --git a/decryptor/postgresql/utils_test.go b/decryptor/postgresql/utils_test.go
index 5473cef95..3a33bbe75 100644
--- a/decryptor/postgresql/utils_test.go
+++ b/decryptor/postgresql/utils_test.go
@@ -62,3 +62,59 @@ func TestParseCommand(t *testing.T) {
 		t.Fatal("parsed and marshaled data not equal")
 	}
 }
+
+func TestWriteParameterArrayEmptyString(t *testing.T) {
+	buffer := bytes.Buffer{}
+	bindValue, err := hex.DecodeString(`0000000100000001000000000000`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	bindPacket, err := NewBindPacket(bindValue)
+	if err != nil {
+		t.Fatal(err)
+	}
+	length, err := writeParameterArray(&buffer, bindPacket.paramValues)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if length != 6 {
+		t.Fatal(err)
+	}
+	// 1 parameter
+	if !bytes.Equal(buffer.Bytes()[:2], []byte{0, 1}) {
+		t.Fatal("Unexpected length of parameters")
+	}
+
+	// one parameter with empty string
+	if !bytes.Equal(buffer.Bytes()[2:], []byte{0, 0, 0, 0}) {
+		t.Fatal("Empty")
+	}
+}
+
+func TestWriteParameterArrayNullValue(t *testing.T) {
+	buffer := bytes.Buffer{}
+	bindValue, err := hex.DecodeString(`0000000100000001ffffffff0000`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	bindPacket, err := NewBindPacket(bindValue)
+	if err != nil {
+		t.Fatal(err)
+	}
+	length, err := writeParameterArray(&buffer, bindPacket.paramValues)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if length != 6 {
+		t.Fatal(err)
+	}
+	// 1 parameter
+	if !bytes.Equal(buffer.Bytes()[:2], []byte{0, 1}) {
+		t.Fatal("Unexpected length of parameters")
+	}
+
+	// one parameter with null value as -1 or 0xffffffff
+	if !bytes.Equal(buffer.Bytes()[2:], []byte{255, 255, 255, 255}) {
+		t.Fatal("Empty")
+	}
+}
diff --git a/tests/test.py b/tests/test.py
index d9ccc7630..d0d286aae 100644
--- a/tests/test.py
+++ b/tests/test.py
@@ -9283,6 +9283,7 @@ def testClientIDRead(self):
                 continue
             if 'empty' in column:
                 self.assertEqual(row[column], '')
+                self.assertEqual(row[column], data[column])
                 continue
             self.assertNotEqual(data[column], row[column])
             if column in ('value_int32', 'value_int64'):
@@ -9443,21 +9444,18 @@ def testClientIDRead(self):
             .where(self.test_table.c.id == sa.bindparam('id')), {'id': data['id']})
         row = self.executor1.execute_prepared_statement(query, args)[0]
         for column in columns:
-            if 'null' in column:
-                # asyncpg decodes None values as empty str/bytes value
-                self.assertFalse(row[column])
-                continue
             self.assertEqual(data[column], row[column])
             self.assertIsInstance(row[column], type(data[column]))
 
         row = self.executor2.execute_prepared_statement(query, args)[0]
         for column in columns:
             if 'null' in column:
-                # asyncpg decodes None values as empty str/bytes value
-                self.assertFalse(row[column])
+                self.assertIsNone(row[column])
+                self.assertIsNone(data[column])
                 continue
             if 'empty' in column:
                 self.assertEqual(data[column], row[column])
+                self.assertEqual(data[column], '')
             else:
                 self.assertNotEqual(data[column], row[column])
                 self.assertEqual(row[column], default_expected_values[column])
@@ -9465,9 +9463,13 @@ def testClientIDRead(self):
 
         row = self.executor2.execute_prepared_statement(query, args)[0]
         for column in columns:
-            if 'null' in column or 'empty' in column:
-                # asyncpg decodes None values as empty str/bytes value
-                self.assertFalse(row[column])
+            if 'null' in column:
+                self.assertIsNone(row[column])
+                self.assertEqual(data[column], row[column])
+                continue
+            if 'empty' in column:
+                self.assertEqual(row[column], '')
+                self.assertEqual(row[column], data[column])
                 continue
             self.assertNotEqual(data[column], row[column])
 
@@ -9520,9 +9522,12 @@ def testClientIDRead(self):
 
         for column in columns:
             if 'empty' in column:
-                self.assertEqual(data[column], row[column])
+                self.assertEqual(row[column], '')
+                self.assertEqual(row[column], data[column])
+                continue
             elif 'null' in column:
                 self.assertEqual(data[column], row[column])
+                self.assertIsNone(data[column])
             else:
                 self.assertNotEqual(data[column], row[column])
                 self.assertEqual(default_expected_values[column], row[column])
@@ -9596,11 +9601,14 @@ def testClientIDRead(self):
                 .where(self.test_table.c.id == data['id']))
         row = result.fetchone()
         for column in columns:
-            if 'null' in column or 'empty' in column:
-                # asyncpg decodes None values as empty str/bytes value
-                self.assertFalse(row[column])
+            if 'null' in column:
+                self.assertIsNone(row[column])
+                self.assertEqual(row[column], data[column])
                 continue
             value = utils.memoryview_to_bytes(row[column])
+            if 'empty' in column:
+                self.assertEqual(value, b'')
+                continue
             self.assertIsInstance(value, bytes, column)
             self.assertNotEqual(data[column], value, column)
 
@@ -9760,14 +9768,18 @@ def testClientIDRead(self):
 
         row = self.raw_executor.execute_prepared_statement(query, args)[0]
         for column in columns:
-            if 'null' in column or 'empty' in column:
-                # asyncpg decodes None values as empty str/bytes value
-                self.assertFalse(row[column])
+            if 'null' in column:
+                self.assertIsNone(row[column])
+                self.assertEqual(row[column], data[column])
                 continue
             value = utils.memoryview_to_bytes(row[column])
+            if 'empty' in column:
+                self.assertEqual(value, b'')
+                continue
             self.assertIsInstance(value, bytes, column)
             self.assertNotEqual(data[column], value, column)
 
+
 class TestPostgresqlBinaryTypeAwareDecryptionWithError(TestPostgresqlBinaryFormatTypeAwareDecryptionWithDefaults):
     # test table used for queries and data mapping into python types
     test_table = sa.Table(
@@ -9838,11 +9850,14 @@ def testClientIDRead(self):
 
         row = self.raw_executor.execute_prepared_statement(query, args)[0]
         for column in columns:
-            if 'null' in column or 'empty' in column:
-                # asyncpg decodes None values as empty str/bytes value
-                self.assertFalse(row[column])
+            if 'null' in column:
+                self.assertIsNone(row[column])
+                self.assertEqual(row[column], data[column])
                 continue
             value = utils.memoryview_to_bytes(row[column])
+            if 'empty' in column:
+                self.assertEqual(value, b'')
+                continue
             self.assertIsInstance(value, bytes, column)
             self.assertNotEqual(data[column], value, column)
 
@@ -9915,11 +9930,14 @@ def testClientIDRead(self):
                 .where(self.test_table.c.id == data['id']))
         row = result.fetchone()
         for column in columns:
-            if 'null' in column or 'empty' in column:
-                # asyncpg decodes None values as empty str/bytes value
-                self.assertFalse(row[column])
+            if 'null' in column:
+                self.assertIsNone(row[column])
+                self.assertEqual(row[column], data[column])
                 continue
             value = utils.memoryview_to_bytes(row[column])
+            if 'empty' in column:
+                self.assertEqual(value, b'')
+                continue
             self.assertIsInstance(value, bytes, column)
             self.assertNotEqual(data[column], value, column)
 
@@ -9993,14 +10011,18 @@ def testClientIDRead(self):
 
         row = self.raw_executor.execute_prepared_statement(query, args)[0]
         for column in columns:
-            if 'null' in column or 'empty' in column:
-                # asyncpg decodes None values as empty str/bytes value
-                self.assertFalse(row[column])
+            if 'null' in column:
+                self.assertIsNone(row[column])
+                self.assertEqual(row[column], data[column])
                 continue
             value = utils.memoryview_to_bytes(row[column])
+            if 'empty' in column:
+                self.assertEqual(value, b'')
+                continue
             self.assertIsInstance(value, bytes, column)
             self.assertNotEqual(data[column], value, column)
 
+
 # `response_on_fail` is `ciphertext` if not defined. That's why the code is
 # exactly the same as inTestPostgresqlTextTypeAwareDecryptionWithoutDefaults
 class TestPostgresqlTextTypeAwareDecryptionWithCiphertext(BaseTransparentEncryption):
@@ -10069,11 +10091,14 @@ def testClientIDRead(self):
                 .where(self.test_table.c.id == data['id']))
         row = result.fetchone()
         for column in columns:
-            if 'null' in column or 'empty' in column:
-                # asyncpg decodes None values as empty str/bytes value
-                self.assertFalse(row[column])
+            if 'null' in column:
+                self.assertIsNone(row[column])
+                self.assertEqual(row[column], data[column])
                 continue
             value = utils.memoryview_to_bytes(row[column])
+            if 'empty' in column:
+                self.assertEqual(value, b'')
+                continue
             self.assertIsInstance(value, bytes, column)
             self.assertNotEqual(data[column], value, column)