From 3c62db26969d5dc89f3510f052664a99ec6a2ca9 Mon Sep 17 00:00:00 2001
From: matejcik <ja@matejcik.cz>
Date: Tue, 1 Oct 2019 14:44:21 +0200
Subject: [PATCH] stellar: fix ManageDataOp value padding

---
 core/src/apps/stellar/operations/serialize.py    |  3 +--
 core/src/apps/stellar/writers.py                 | 11 +++++++++--
 legacy/firmware/stellar.c                        |  4 +---
 .../test_msg_stellar_sign_transaction.py         | 16 ++++++++++++++--
 4 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/core/src/apps/stellar/operations/serialize.py b/core/src/apps/stellar/operations/serialize.py
index d7fa4af624..10b34f38c7 100644
--- a/core/src/apps/stellar/operations/serialize.py
+++ b/core/src/apps/stellar/operations/serialize.py
@@ -56,8 +56,7 @@ def write_manage_data_op(w, msg: StellarManageDataOp):
     writers.write_string(w, msg.key)
     writers.write_bool(w, bool(msg.value))
     if msg.value:
-        writers.write_uint32(w, len(msg.value))
-        writers.write_bytes(w, msg.value)
+        writers.write_string(w, msg.value)
 
 
 def write_manage_offer_op(w, msg: StellarManageOfferOp):
diff --git a/core/src/apps/stellar/writers.py b/core/src/apps/stellar/writers.py
index f6c651400a..0846d148fc 100644
--- a/core/src/apps/stellar/writers.py
+++ b/core/src/apps/stellar/writers.py
@@ -5,9 +5,16 @@ from apps.common.writers import write_bytes, write_uint32_be, write_uint64_be
 write_uint32 = write_uint32_be
 write_uint64 = write_uint64_be
 
+if False:
+    from typing import AnyStr
 
-def write_string(w, s: str):
-    buf = s.encode()
+
+def write_string(w, s: AnyStr) -> None:
+    """Write XDR string padded to a multiple of 4 bytes."""
+    if isinstance(s, str):
+        buf = s.encode()
+    else:
+        buf = s
     write_uint32(w, len(buf))
     write_bytes(w, buf)
     # if len isn't a multiple of 4, add padding bytes
diff --git a/legacy/firmware/stellar.c b/legacy/firmware/stellar.c
index bf304a0349..f8833b4c0f 100644
--- a/legacy/firmware/stellar.c
+++ b/legacy/firmware/stellar.c
@@ -1088,9 +1088,7 @@ bool stellar_confirmManageDataOp(const StellarManageDataOp *msg) {
   // value
   if (msg->has_value) {
     stellar_hashupdate_bool(true);
-    // Variable opaque field is length + raw bytes
-    stellar_hashupdate_uint32(msg->value.size);
-    stellar_hashupdate_bytes(msg->value.bytes, msg->value.size);
+    stellar_hashupdate_string(msg->value.bytes, msg->value.size);
   } else {
     stellar_hashupdate_bool(false);
   }
diff --git a/tests/device_tests/test_msg_stellar_sign_transaction.py b/tests/device_tests/test_msg_stellar_sign_transaction.py
index 564e452b70..7d36b0e7c1 100644
--- a/tests/device_tests/test_msg_stellar_sign_transaction.py
+++ b/tests/device_tests/test_msg_stellar_sign_transaction.py
@@ -55,7 +55,6 @@ from trezorlib.tools import parse_path
 
 from ..common import MNEMONIC12
 
-
 pytestmark = [
     pytest.mark.altcoin,
     pytest.mark.stellar,
@@ -66,7 +65,7 @@ ADDRESS_N = parse_path(stellar.DEFAULT_BIP32_PATH)
 NETWORK_PASSPHRASE = "Test SDF Network ; September 2015"
 
 
-def _create_msg(self) -> messages.StellarSignTx:
+def _create_msg() -> messages.StellarSignTx:
     return messages.StellarSignTx(
         source_account="GAK5MSF74TJW6GLM7NLTL76YZJKM2S4CGP3UH4REJHPHZ4YBZW2GSBPW",
         fee=100,
@@ -260,3 +259,16 @@ def test_sign_tx_set_options(client):
         b64encode(response.signature)
         == b"22rfcOrxBiE5akpNsnWX8yPgAOpclbajVqXUaXMNeL000p1OhFhi050t1+GNRpoSNyfVsJGNvtlICGpH4ksDAQ=="
     )
+
+
+def test_manage_data(client):
+    tx = _create_msg()
+
+    # testing a value whose length is not divisible by 4
+    op = messages.StellarManageDataOp(key="data", value=b"abc")
+
+    response = stellar.sign_tx(client, tx, [op], ADDRESS_N, NETWORK_PASSPHRASE)
+    assert (
+        b64encode(response.signature)
+        == b"MTa8fmhM7BxYYo9kY1RKQFFrg/MVfzsgiPSuP7+SJZjhQ3P1ucN5JUnfmD5484ULRPjVTd+0YYjS6hScLupbCQ=="
+    )