coze-dev / coze-dev/coze-studio

Cross-tenant read, write and delete of any workspace's Database (memory table) data via the workflow SQL Customization node

Open
#2,710 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
21.6k
Forks
3.1k
PR merge metrics
No merged PRs in 30d

Description

version: commit 22275b1c2661d35344a7493cffe401e8cc61cf8e

### Summary

Any authenticated user of Coze Studio can read, insert into, and delete rows from the "Database" (memory table) resource of any other workspace, including workspaces they have no membership in or relationship to. The workflow builder's "SQL Customization" node lets a user write a literal SQL statement that is executed almost as-is against the underlying MySQL/RDS backend. The only server-side validation is a keyword blocklist and a regex that requires referenced table names to look like `table_` - the exact naming pattern used for every memory-database physical table across the entire multi-tenant deployment. There is no check that the table named in the SQL text is the one actually bound to the request. An attacker only needs to guess or enumerate another tenant's physical table id (a predictable, sequentially-generated numeric id) to read, corrupt, or delete that tenant's data.

### Details

The "Database" feature (`backend/domain/memory/database`) lets a user attach a structured table to a bot/workflow. Every such table is backed by an underlying MySQL table named `table_`, where `` comes from a global ID generator shared by the whole install (`backend/infra/rdb/impl/rdb/mysql.go:825`):

```go
func (m *mysqlService) genTableName(...) ... {
id, err := m.generator.GenID(ctx)
...
return fmt.Sprintf("table_%d", id), nil
}
```

The workflow "SQL Customization" node (`backend/domain/workflow/internal/nodes/database/customsql.go`) lets any workspace member who can build a workflow write a raw SQL template bound to one of their own Database resources (`DatabaseInfoID`). After variable substitution it is sent unmodified to `crossdomain/database`:

```go
// customsql.go:170-172
req.SQL = templateSQL
response, err := crossdatabase.DefaultSVC().Execute(ctx, req)
```

`Execute()` (`backend/crossdomain/database/impl/database.go:79`) forwards it straight into `ExecuteSQL` with `OperateType_Custom`, keyed only by the caller's own `databaseInfoID` - there is no check anywhere in this path that the SQL text actually references that database's table.

`executeCustomSQL()` (`backend/domain/memory/database/service/database_impl.go:1044`) is the only place a check is made, and it was patched three times in a row (commits `013bdab1`, `4e819f6d`, `f1ccf68e`) after prior SQL-injection reports:

```go
// database_impl.go:1050
if err := validateCustomSQL(*req.SQL); err != nil { ... } // keyword blocklist (UNION/JOIN/system tables/dangerous funcs)
...
// database_impl.go:1074-1088
tableColumnMapping := map[string]sqlparsercontract.TableColumn{
tableInfo.TableName: { // only the CALLER'S OWN logical table name is mapped
NewTableName: &physicalTableName,
ColumnMap: fieldNameToPhysical,
},
}
parsedSQL, err := sqlparser.New().ParseAndModifySQL(*req.SQL, tableColumnMapping)
...
if err := validateParsedSQL(parsedSQL); err != nil { ... } // table-name FORMAT check only
```

`validateParsedSQL` (`database_impl.go:2238`) and the pattern it checks against (`database_impl.go:2203`):

```go
var allowedTableNamePattern = regexp.MustCompile(`^table_\d+$`)

func validateParsedSQL(parsedSQL string) error {
tableNamePattern := regexp.MustCompile(`(?i)\b(FROM|JOIN|INTO|UPDATE)\s+` + "`?" + `(\w+)` + "`?")
matches := tableNamePattern.FindAllStringSubmatch(parsedSQL, -1)
for _, match := range matches {
tableName := match[2]
if tableName == "dual" { continue }
if !allowedTableNamePattern.MatchString(tableName) {
return fmt.Errorf("invalid table name: %s, only table_ format is allowed", tableName)
}
}
return nil
}
```

This only verifies the table name *looks like* `table_`. It never compares the matched name to `physicalTableName` (the table actually authorized for the caller's `DatabaseInfoID`). `sqlparser.ParseAndModifySQL` (`backend/infra/sqlparser/impl/sqlparser/sql_parser.go:158-166`) only rewrites a `TableName` AST node if it exactly equals the caller's own logical table name (`tableInfo.TableName`); any other table name in the SQL text - including a literal `table_` - is left completely untouched and executed as written.

The row-scoping filter that exists for "single user mode" databases is likewise irrelevant to this bypass, because it is keyed off the *caller's own* selected database's `RwMode`, not the table actually referenced in the SQL text (`database_impl.go:1090`):

```go
if tableInfo.RwMode == table.BotTableRWMode_LimitedReadWrite && len(req.UserID) != 0 {
switch operation {
case Select, Update, Delete:
parsedSQL, _ = sqlparser.New().AppendSQLFilter(parsedSQL, And, fmt.Sprintf("uid = '%s'", req.UserID))
}
}
```

An attacker simply creates (or already owns) a throwaway database with RW mode "Unlimited Read-Write" (`rw_mode: 3`, settable directly via `POST /api/memory/database/add` or `/update`) so this filter never triggers, then references any other tenant's `table_` directly in the SQL text. INSERT is not covered by this filter at all regardless of RW mode, so cross-tenant writes work unconditionally.

Net effect: any authenticated user can read, insert, or delete rows in any other tenant's Database resource, as long as they can name (guess/enumerate) that resource's physical `table_`, which is a small, sequentially-generated numeric id with no per-tenant namespacing or access check anywhere on this path.

### PoC

(available upon request)

### Summary

Any authenticated user of Coze Studio can read, insert into, and delete rows from the "Database" (memory table) resource of any other workspace, including workspaces they have no membership in or relationship to. The workflow builder's "SQL Customization" node lets a user write a literal SQL statement that is executed almost as-is against the underlying MySQL/RDS backend. The only server-side validation is a keyword blocklist and a regex that requires referenced table names to look like `table_` - the exact naming pattern used for every memory-database physical table across the entire multi-tenant deployment. There is no check that the table named in the SQL text is the one actually bound to the request. An attacker only needs to guess or enumerate another tenant's physical table id (a predictable, sequentially-generated numeric id) to read, corrupt, or delete that tenant's data.

### Details

The "Database" feature (`backend/domain/memory/database`) lets a user attach a structured table to a bot/workflow. Every such table is backed by an underlying MySQL table named `table_`, where `` comes from a global ID generator shared by the whole install (`backend/infra/rdb/impl/rdb/mysql.go:825`):

```go
func (m *mysqlService) genTableName(...) ... {
id, err := m.generator.GenID(ctx)
...
return fmt.Sprintf("table_%d", id), nil
}
```

The workflow "SQL Customization" node (`backend/domain/workflow/internal/nodes/database/customsql.go`) lets any workspace member who can build a workflow write a raw SQL template bound to one of their own Database resources (`DatabaseInfoID`). After variable substitution it is sent unmodified to `crossdomain/database`:

```go
// customsql.go:170-172
req.SQL = templateSQL
response, err := crossdatabase.DefaultSVC().Execute(ctx, req)
```

`Execute()` (`backend/crossdomain/database/impl/database.go:79`) forwards it straight into `ExecuteSQL` with `OperateType_Custom`, keyed only by the caller's own `databaseInfoID` - there is no check anywhere in this path that the SQL text actually references that database's table.

`executeCustomSQL()` (`backend/domain/memory/database/service/database_impl.go:1044`) is the only place a check is made, and it was patched three times in a row (commits `013bdab1`, `4e819f6d`, `f1ccf68e`) after prior SQL-injection reports:

```go
// database_impl.go:1050
if err := validateCustomSQL(*req.SQL); err != nil { ... } // keyword blocklist (UNION/JOIN/system tables/dangerous funcs)
...
// database_impl.go:1074-1088
tableColumnMapping := map[string]sqlparsercontract.TableColumn{
tableInfo.TableName: { // only the CALLER'S OWN logical table name is mapped
NewTableName: &physicalTableName,
ColumnMap: fieldNameToPhysical,
},
}
parsedSQL, err := sqlparser.New().ParseAndModifySQL(*req.SQL, tableColumnMapping)
...
if err := validateParsedSQL(parsedSQL); err != nil { ... } // table-name FORMAT check only
```

`validateParsedSQL` (`database_impl.go:2238`) and the pattern it checks against (`database_impl.go:2203`):

```go
var allowedTableNamePattern = regexp.MustCompile(`^table_\d+$`)

func validateParsedSQL(parsedSQL string) error {
tableNamePattern := regexp.MustCompile(`(?i)\b(FROM|JOIN|INTO|UPDATE)\s+` + "`?" + `(\w+)` + "`?")
matches := tableNamePattern.FindAllStringSubmatch(parsedSQL, -1)
for _, match := range matches {
tableName := match[2]
if tableName == "dual" { continue }
if !allowedTableNamePattern.MatchString(tableName) {
return fmt.Errorf("invalid table name: %s, only table_ format is allowed", tableName)
}
}
return nil
}
```

This only verifies the table name *looks like* `table_`. It never compares the matched name to `physicalTableName` (the table actually authorized for the caller's `DatabaseInfoID`). `sqlparser.ParseAndModifySQL` (`backend/infra/sqlparser/impl/sqlparser/sql_parser.go:158-166`) only rewrites a `TableName` AST node if it exactly equals the caller's own logical table name (`tableInfo.TableName`); any other table name in the SQL text - including a literal `table_` - is left completely untouched and executed as written.

The row-scoping filter that exists for "single user mode" databases is likewise irrelevant to this bypass, because it is keyed off the *caller's own* selected database's `RwMode`, not the table actually referenced in the SQL text (`database_impl.go:1090`):

```go
if tableInfo.RwMode == table.BotTableRWMode_LimitedReadWrite && len(req.UserID) != 0 {
switch operation {
case Select, Update, Delete:
parsedSQL, _ = sqlparser.New().AppendSQLFilter(parsedSQL, And, fmt.Sprintf("uid = '%s'", req.UserID))
}
}
```

An attacker simply creates (or already owns) a throwaway database with RW mode "Unlimited Read-Write" (`rw_mode: 3`, settable directly via `POST /api/memory/database/add` or `/update`) so this filter never triggers, then references any other tenant's `table_` directly in the SQL text. INSERT is not covered by this filter at all regardless of RW mode, so cross-tenant writes work unconditionally.

Net effect: any authenticated user can read, insert, or delete rows in any other tenant's Database resource, as long as they can name (guess/enumerate) that resource's physical `table_`, which is a small, sequentially-generated numeric id with no per-tenant namespacing or access check anywhere on this path.

### PoC

Contributor guide

Open the contributing guide

Research direction

Start by tracing executeCustomSQL in backend/domain/memory/database/service/database_impl.go, then follow Execute in backend/crossdomain/database/impl/database.go and the SQL node in backend/domain/workflow/internal/nodes/database/customsql.go. Review ParseAndModifySQL in backend/infra/sqlparser/impl/sqlparser/sql_parser.go and verify that SQL cannot target a physical table other than the authorized Database resource; done means cross-workspace reads, inserts, and deletes are rejected.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, mysql
Domain
authorization, backend-api-design, databases, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.