Ethan-Xingyue opened a new issue, #1157:
URL: https://github.com/apache/incubator-seata-go/issues/1157

   
   | Field | Value |
   | --- | --- |
   | Issue title | `[BUG] [TCC] Malformed actionContext in applicationData 
panics during branch commit/rollback` |
   | Labels (exist in repo) | `bug`, `module/tcc`, `module/rm` |
   | Suggested priority | P1 (network-boundary robustness) |
   | Related | none known |
   | Verification | Reproduced 2026-09-02 on master `3bf73586` with go1.24.3 
darwin/arm64 |
   | Before publishing | Decide whether the `applicationData` field can be 
controlled by an untrusted TC or an on-path attacker in your threat model. If 
yes, report through [email protected] first instead of a public issue. |
   
   ---
   
   ## 🚀 Go Version
   
   go1.24.3 darwin/arm64
   
   ## 📦 Seata-go Version
   
   master, commit 3bf73586af81db1bd428982c93d82000d80cb1c8 (fetched 2026-09-02)
   
   ## 💾 Operating System
   
   macOS
   
   ## 📝 Bug Description
   
   TCC branch commit and rollback parse the `applicationData` carried by the 
branch request in `TCCResourceManager.getBusinessActionContext`:
   
   
https://github.com/apache/incubator-seata-go/blob/3bf73586af81db1bd428982c93d82000d80cb1c8/pkg/rm/tcc/tcc_resource.go#L152-L169
   
   Two inputs turn into a process panic instead of a failure response:
   
   - Invalid JSON: `json.Unmarshal` error is converted into `panic("application 
data failed to unmarshl as json")` (L157).
   - Valid JSON whose `actionContext` is not an object (string, number, array, 
`null`): the unchecked type assertion `v.(map[string]interface{})` (L160) 
panics.
   
   Nothing on the processor or listener path recovers from this panic and 
converts it into a failed branch response, so one malformed message can take 
down the RM client.
   
   ## 🔄 Steps to Reproduce
   
   1. Check out the commit and create a throwaway module that points at the 
local checkout:
   
   ```bash
   git clone https://github.com/apache/incubator-seata-go.git
   cd incubator-seata-go
   git checkout 3bf73586af81db1bd428982c93d82000d80cb1c8
   repo_root="$(pwd)"
   repro_dir="$(mktemp -d)"
   cd "$repro_dir"
   go mod init seata-repro
   go mod edit -require=seata.apache.org/seata-go/[email protected]
   go mod edit -replace=seata.apache.org/seata-go/v2="$repo_root"
   ```
   
   2. Save the following as `repro_test.go` in `$repro_dir`:
   
   ```go
   package repro
   
   import (
        "context"
        "testing"
   
        "seata.apache.org/seata-go/v2/pkg/protocol/branch"
        "seata.apache.org/seata-go/v2/pkg/rm"
        "seata.apache.org/seata-go/v2/pkg/rm/tcc"
        "seata.apache.org/seata-go/v2/pkg/tm"
   )
   
   // noOpRMRemoting avoids any network access when registering the TCC 
resource.
   type noOpRMRemoting struct{}
   
   func (noOpRMRemoting) BranchRegister(rm.BranchRegisterParam) (int64, error) 
{ return 1, nil }
   func (noOpRMRemoting) BranchReport(rm.BranchReportParam) error              
{ return nil }
   func (noOpRMRemoting) LockQuery(rm.LockQueryParam) (bool, error)            
{ return true, nil }
   func (noOpRMRemoting) RegisterResource(rm.Resource) error                   
{ return nil }
   
   type tccAction struct{}
   
   func (*tccAction) Prepare(context.Context, interface{}) (bool, error) { 
return true, nil }
   func (*tccAction) Commit(context.Context, *tm.BusinessActionContext) (bool, 
error) {
        return true, nil
   }
   func (*tccAction) Rollback(context.Context, *tm.BusinessActionContext) 
(bool, error) {
        return true, nil
   }
   func (*tccAction) GetActionName() string { return "audit-tcc" }
   
   func TestMalformedTCCApplicationDataReturnsErrorInsteadOfPanicking(t 
*testing.T) {
        rm.SetRMRemotingInstance(noOpRMRemoting{})
        resource, err := tcc.ParseTCCResource(&tccAction{})
        if err != nil {
                t.Fatal(err)
        }
        manager := tcc.GetTCCResourceManagerInstance()
        if err := manager.RegisterResource(resource); err != nil {
                t.Fatal(err)
        }
   
        defer func() {
                if r := recover(); r != nil {
                        t.Fatalf("malformed applicationData panicked: %v", r)
                }
        }()
        _, err = manager.BranchCommit(context.Background(), rm.BranchResource{
                BranchType:      branch.BranchTypeTCC,
                Xid:             "xid-1",
                BranchId:        1,
                ResourceId:      "audit-tcc",
                ApplicationData: []byte(`{"actionContext":"not-an-object"}`),
        })
        if err == nil {
                t.Fatal("malformed applicationData was accepted")
        }
   }
   ```
   
   3. Run:
   
   ```bash
   go mod tidy
   go test -run 
'^TestMalformedTCCApplicationDataReturnsErrorInsteadOfPanicking$' -count=1 -v
   ```
   
   ## ✅ Expected Behavior
   
   No panic. `BranchCommit` / `BranchRollback` return a typed or wrapped error 
stating that `actionContext` must be a JSON object (and that `applicationData` 
must be valid JSON), and the branch processor turns that error into a failed 
branch response for the TC.
   
   ## ❌ Actual Behavior
   
   ```text
   === RUN   TestMalformedTCCApplicationDataReturnsErrorInsteadOfPanicking
       repro_test.go:45: malformed applicationData panicked: interface 
conversion: interface {} is string, not map[string]interface {}
   --- FAIL: TestMalformedTCCApplicationDataReturnsErrorInsteadOfPanicking 
(0.00s)
   FAIL
   FAIL seata-audit-repros/rm002        0.629s
   FAIL
   ```
   
   ## 💡 Possible Solution
   
   - Change `getBusinessActionContext` to return `(*tm.BusinessActionContext, 
error)`; validate the top-level JSON, the `actionContext` type and reasonable 
size limits.
   - Let the branch processors map the error to a failed response; keep a 
`recover` at the listener level only as a last line of defence, not as input 
validation.
   - Add table-driven and fuzz tests for invalid JSON, string / number / array 
/ null `actionContext`, and oversized objects.
   
   Acceptance criteria:
   
   - [ ] The test above passes; invalid JSON and every non-object type also do 
not panic.
   - [ ] Getty and gRPC paths for both commit and rollback return a 
deterministic failure response.
   - [ ] A fuzz test runs for the agreed time without panic, OOM or indefinite 
blocking.
   - [ ] Existing valid payloads stay wire-compatible.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to