T09 · Insecure Skill Coding Practices
Error
- Location
- assets/dbcore/base_model.go:132
- Finding
- Incomplete Multi-Tenant Write Isolation Allows Tenant Spoofing and Reassignment<![CDATA[ ## Vulnerability Details **File Location**: `assets/dbcore/base_model.go:132-180` **Vulnerability Type**: Incomplete tenant authorization enforcement **Risk Level**: High ### Vulnerable Code ```go // GetTxDB 获取数据库连接（优先从 ctx 中取事务连接） // 当 TenantEnabled=true 时，自动注入租户隔离条件 func (m *BaseModel[T]) GetTxDB(ctx context.Context) *gorm.DB { db := GetDB(ctx, m.DB).WithContext(ctx) if globalConfig.TenantEnabled && globalConfig.TenantExtractor != nil { tenantID := globalConfig.TenantExtractor(ctx) if tenantID != "" { db = db.Where(safeField(globalConfig.TenantField)+" = ?", tenantID) } else if globalConfig.TenantStrict { // 严格模式：无租户信息时拒绝所有查询，防止数据越权 db = db.Where("1 = 0") } } return db } // ==================== 插入操作 ==================== func (m *BaseModel[T]) Insert(ctx context.Context, v *T) error { autoFillID(v) return m.GetTxDB(ctx).Create(v).Error } func (m *BaseModel[T]) InsertBatch(ctx context.Context, v []*T, batchSize int) error { if len(v) == 0 { return nil } autoFillIDBatch(v) if batchSize <= 0 { batchSize = 100 } return m.GetTxDB(ctx).CreateInBatches(v, batchSize).Error } // ==================== 更新操作 ==================== func (m *BaseModel[T]) Update(ctx context.Context, id string, v map[string]interface{}) error { if id == "" { return gorm.ErrMissingWhereClause } return m.GetTxDB(ctx).Model(new(T)).Where("id = ?", id).Updates(v).Error } func (m *BaseModel[T]) UpdateBy(ctx context.Context, v map[string]interface{}, query string, args ...interface{}) error { if query == "" { return gorm.ErrMissingWhereClause } return m.GetTxDB(ctx).Model(new(T)).Where(query, args...).Updates(v).Error } ``` ### Technical Analysis Tenant enforcement is implemented exclusively as a GORM `WHERE` condition. That condition restricts which existing rows can be selected, updated, or deleted, but it does not securely control the value written into the tenant column. The `Insert` and `InsertBatch` methods pass caller-provided obj ...[truncated 2853 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate configuration when tenant mode is enabled: - Reject `TenantEnabled: true` if `TenantExtractor` is nil. - In strict mode, return an explicit error when the tenant ID is missing rather than relying only on `WHERE 1 = 0`. 2. Enforce tenant ownership on insert: - Resolve the tenant ID from the trusted context. - Set the configured tenant field on every object before calling `Create` or `CreateInBatches`. - Overwrite any caller-provided tenant value rather than trusting it. - Consider a GORM create callback if reflection-based assignment must support generic models. 3. Protect updates: - Reject update maps containing the configured tenant field. - Alternatively, delete that key from the map and preserve the existing tenant value. - Provide a separate, explicitly privileged administrative operation for legitimate tenant transfers. 4. Apply equivalent protections to full-model writes such as `Save`, which can also persist a caller-controlled tenant field. 5. Prefer immutable tenant ownership at the database layer where practical: - Use database triggers or repository-specific constraints to prevent ordinary operations from changing tenant IDs. - Apply composite uniqueness and indexing rules that include the tenant column. 6. Add security tests covering: - Insert with a spoofed tenant ID. - Batch insert containing mixed tenant IDs. - Update and `Save` attempts that change tenant ownership. - Missing tenant context in strict mode. - `TenantEnabled` with a nil extractor. - Read, update, and delete attempts across two distinct tenants. ]]>