Skip to content

Commit 23f87c2

Browse files
committed
Protect operations database from non-string data in error messages from reverts
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
1 parent df3fe81 commit 23f87c2

File tree

2 files changed

+125
-1
lines changed

2 files changed

+125
-1
lines changed

internal/operations/operation_updater.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,11 @@ package operations
1818

1919
import (
2020
"context"
21+
"encoding/hex"
2122
"fmt"
23+
"strings"
2224
"time"
25+
"unicode/utf8"
2326

2427
"github.com/hyperledger/firefly-common/pkg/config"
2528
"github.com/hyperledger/firefly-common/pkg/ffapi"
@@ -422,7 +425,15 @@ func (ou *operationUpdater) resolveOperation(ctx context.Context, ns string, id
422425
update = update.Set("status", status)
423426
}
424427
if errorMsg != nil {
425-
update = update.Set("error", *errorMsg)
428+
// PostgreSQL text columns reject null bytes and invalid UTF-8 sequences.
429+
// Null bytes (0x00) are valid UTF-8 but rejected by PostgreSQL, so check both.
430+
if !utf8.ValidString(*errorMsg) || strings.ContainsRune(*errorMsg, 0) {
431+
hexString := hex.EncodeToString([]byte(*errorMsg))
432+
log.L(ctx).Warnf("Error message contains invalid UTF-8 or null bytes - encoding as hex: %s", hexString)
433+
update = update.Set("error", hexString)
434+
} else {
435+
update = update.Set("error", *errorMsg)
436+
}
426437
}
427438
if output != nil {
428439
update = update.Set("output", output)

internal/operations/operation_updater_test.go

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,119 @@ func TestDoUpdateVerifyBatchManifestFail(t *testing.T) {
541541
mdi.AssertExpectations(t)
542542
}
543543

544+
func TestResolveOperationValidUTF8ErrorPassesThrough(t *testing.T) {
545+
ou := newTestOperationUpdaterNoConcurrency(t)
546+
defer ou.close()
547+
548+
opID1 := fftypes.NewUUID()
549+
mdi := ou.database.(*databasemocks.Plugin)
550+
mdi.On("GetOperations", mock.Anything, mock.Anything, mock.Anything).Return([]*core.Operation{
551+
{ID: opID1, Namespace: "ns1", Type: core.OpTypeBlockchainInvoke},
552+
}, nil, nil)
553+
mdi.On("UpdateOperation", mock.Anything, "ns1", opID1, mock.Anything, mock.MatchedBy(updateMatcher([][]string{
554+
{"status", "Failed"},
555+
{"error", "FF23021: EVM reverted: some normal error message"},
556+
}))).Return(true, nil)
557+
558+
ou.initQueues()
559+
560+
err := ou.doBatchUpdate(ou.ctx, []*core.OperationUpdate{
561+
{NamespacedOpID: "ns1:" + opID1.String(), Status: core.OpStatusFailed, ErrorMessage: "FF23021: EVM reverted: some normal error message"},
562+
})
563+
assert.NoError(t, err)
564+
565+
mdi.AssertExpectations(t)
566+
}
567+
568+
func TestResolveOperationInvalidUTF8ErrorHexEncoded(t *testing.T) {
569+
ou := newTestOperationUpdaterNoConcurrency(t)
570+
defer ou.close()
571+
572+
opID1 := fftypes.NewUUID()
573+
574+
// Simulate the actual revert scenario: readable text with embedded ABI-encoded Error(string)
575+
// selector bytes (0x08, 0xc3, 0x79, 0xa0) and null byte padding, which is invalid UTF-8
576+
invalidMsg := "[OCPE]404/98 - \x08\xc3\x79\xa0\x00\x00\x00[TMM]404/16e"
577+
expectedHex := "5b4f4350455d3430342f3938202d2008c379a00000005b544d4d5d3430342f313665"
578+
579+
mdi := ou.database.(*databasemocks.Plugin)
580+
mdi.On("GetOperations", mock.Anything, mock.Anything, mock.Anything).Return([]*core.Operation{
581+
{ID: opID1, Namespace: "ns1", Type: core.OpTypeBlockchainInvoke},
582+
}, nil, nil)
583+
mdi.On("UpdateOperation", mock.Anything, "ns1", opID1, mock.Anything, mock.MatchedBy(updateMatcher([][]string{
584+
{"status", "Failed"},
585+
{"error", expectedHex},
586+
}))).Return(true, nil)
587+
588+
ou.initQueues()
589+
590+
err := ou.doBatchUpdate(ou.ctx, []*core.OperationUpdate{
591+
{NamespacedOpID: "ns1:" + opID1.String(), Status: core.OpStatusFailed, ErrorMessage: invalidMsg},
592+
})
593+
assert.NoError(t, err)
594+
595+
mdi.AssertExpectations(t)
596+
}
597+
598+
func TestResolveOperationNullBytesOnlyInvalidUTF8(t *testing.T) {
599+
ou := newTestOperationUpdaterNoConcurrency(t)
600+
defer ou.close()
601+
602+
opID1 := fftypes.NewUUID()
603+
604+
// Null bytes mixed with non-continuation bytes that break UTF-8 validity
605+
invalidMsg := "error\x00with\x80null"
606+
expectedHex := "6572726f720077697468806e756c6c"
607+
608+
mdi := ou.database.(*databasemocks.Plugin)
609+
mdi.On("GetOperations", mock.Anything, mock.Anything, mock.Anything).Return([]*core.Operation{
610+
{ID: opID1, Namespace: "ns1", Type: core.OpTypeBlockchainInvoke},
611+
}, nil, nil)
612+
mdi.On("UpdateOperation", mock.Anything, "ns1", opID1, mock.Anything, mock.MatchedBy(updateMatcher([][]string{
613+
{"status", "Failed"},
614+
{"error", expectedHex},
615+
}))).Return(true, nil)
616+
617+
ou.initQueues()
618+
619+
err := ou.doBatchUpdate(ou.ctx, []*core.OperationUpdate{
620+
{NamespacedOpID: "ns1:" + opID1.String(), Status: core.OpStatusFailed, ErrorMessage: invalidMsg},
621+
})
622+
assert.NoError(t, err)
623+
624+
mdi.AssertExpectations(t)
625+
}
626+
627+
func TestResolveOperationNullBytesInValidUTF8HexEncoded(t *testing.T) {
628+
ou := newTestOperationUpdaterNoConcurrency(t)
629+
defer ou.close()
630+
631+
opID1 := fftypes.NewUUID()
632+
633+
// Pure null bytes embedded in otherwise valid UTF-8 text.
634+
// utf8.ValidString returns true for this, but PostgreSQL rejects 0x00 in text columns.
635+
invalidMsg := "hello\x00world"
636+
expectedHex := "68656c6c6f00776f726c64"
637+
638+
mdi := ou.database.(*databasemocks.Plugin)
639+
mdi.On("GetOperations", mock.Anything, mock.Anything, mock.Anything).Return([]*core.Operation{
640+
{ID: opID1, Namespace: "ns1", Type: core.OpTypeBlockchainInvoke},
641+
}, nil, nil)
642+
mdi.On("UpdateOperation", mock.Anything, "ns1", opID1, mock.Anything, mock.MatchedBy(updateMatcher([][]string{
643+
{"status", "Failed"},
644+
{"error", expectedHex},
645+
}))).Return(true, nil)
646+
647+
ou.initQueues()
648+
649+
err := ou.doBatchUpdate(ou.ctx, []*core.OperationUpdate{
650+
{NamespacedOpID: "ns1:" + opID1.String(), Status: core.OpStatusFailed, ErrorMessage: invalidMsg},
651+
})
652+
assert.NoError(t, err)
653+
654+
mdi.AssertExpectations(t)
655+
}
656+
544657
func TestDoUpdateVerifyBlobManifestFail(t *testing.T) {
545658
ou := newTestOperationUpdaterNoConcurrency(t)
546659
defer ou.close()

0 commit comments

Comments
 (0)