🛡️ Sentinel: [HIGH] Fix SQL injection & Object mutation vulnerability in ERD Model

.jules/sentinel.md

@@ -20,3 +20,8 @@
 **Vulnerability:** The standard user authentication routes (login, register, and reset-password) did not have a maximum length constraint on passwords. This allows an attacker to supply extremely long strings, which `bcrypt` will try to hash, causing CPU exhaustion and creating a Denial of Service (DoS) vulnerability.
 **Learning:** `bcrypt` (and `bcryptjs`) is intentionally slow. While `bcrypt` may internally truncate passwords to 72 bytes, depending on the implementation the input string processing itself or the full string parsing before truncation can be very costly. In this codebase, the admin authentication correctly checked for a max length, but user schemas did not.
 **Prevention:** Always enforce a maximum string length limit (e.g. `.max(1024)`) on user inputs that will be passed into expensive algorithms like bcrypt hashing.
+
+## 2025-05-18 - [ERD Tool SQL Injection and Object Mutation]
+**Vulnerability:** ERD Engineering Tool에서 사용자 입력을 바탕으로 DDL을 생성할 때, 데이터 타입(type)에 대한 허용 목록(allowlist) 검증이 누락되어 SQL 인젝션 공격이 가능했습니다. 또한, 테이블의 내부 상태 객체(Table)가 깊은 복사 없이 외부로 노출되어, 직접 참조를 통해 속성이 변조되는(state mutation) 취약점이 발견되었습니다.
+**Learning:** 객체의 상태가 외부로 유출될 경우, 의도치 않은 변조를 통해 시스템의 무결성이 훼손될 수 있습니다. 특히 SQL과 같은 쿼리 언어의 입력값은 강력한 샌드박싱과 검증이 필요하며, allowlist를 사용하는 것이 안전합니다.
+**Prevention:** `SAFE_SQL_TYPE` 허용 목록 정규식을 도입하여 입력값을 검증하고(`assertSafeSQLType`), `getTable`, `getTables`와 같은 상태 접근 메서드에서는 객체의 깊은 복사본(`structuredClone`)을 반환하도록 하며, `addTable`, `addColumn`에서도 객체 참조를 안전하게 복제하여 원본 객체의 불변성을 유지해야 합니다.

packages/web/src/lib/erd.test.ts

@@ -13,7 +13,7 @@ describe('ERDModel', () => {
       const table = model.addTable('users')
       expect(table.name).toBe('users')
       expect(model.getTables().length).toBe(1)
-      expect(model.getTable('users')).toBe(table)
+      expect(model.getTable('users')).toStrictEqual(table)
     })
 
     it('should throw when adding duplicate table', () => {
@@ -44,6 +44,15 @@ describe('ERDModel', () => {
       expect(table?.columns[0].name).toBe('id')
     })
 
+    it('should prevent mutating internal state after adding a column', () => {
+      model.addTable('users')
+      const column = { name: 'id', type: 'integer' }
+      model.addColumn('users', column)
+      column.type = 'DROP TABLE users;' // Should not affect the model
+      const table = model.getTable('users')
+      expect(table?.columns[0].type).toBe('integer')
+    })
+
     it('should throw when adding a column to a non-existent table', () => {
       expect(() =>
         model.addColumn('non_existent', { name: 'id', type: 'integer' })
@@ -67,6 +76,13 @@ describe('ERDModel', () => {
         model.addColumn('users', { name: 'created__at', type: 'timestamp' })
       ).toThrowError("Column 'created__at' must be snake_case.")
     })
+
+    it('should reject invalid SQL types', () => {
+      model.addTable('users')
+      expect(() =>
+        model.addColumn('users', { name: 'id', type: 'DROP TABLE users;' })
+      ).toThrowError("Invalid SQL type: DROP TABLE users;")
+    })
   })
 
   describe('Foreign Key Management', () => {

packages/web/src/lib/erd.ts

@@ -18,13 +18,20 @@ export interface Table {
 }
 
 const SNAKE_CASE_IDENTIFIER = /^[a-z][a-z0-9]*(?:_[a-z0-9]+)*$/
+const SAFE_SQL_TYPE = /^[A-Za-z0-9_]+(?:\s+[A-Za-z0-9_]+)*(?:\(\s*\d+(?:\s*,\s*\d+)?\s*\))?(?:\s+[A-Za-z0-9_]+)*$/i
 
 function assertSnakeCaseIdentifier(kind: string, name: string): void {
   if (!SNAKE_CASE_IDENTIFIER.test(name)) {
     throw new Error(`${kind} '${name}' must be snake_case.`)
   }
 }
 
+function assertSafeSQLType(type: string): void {
+  if (!SAFE_SQL_TYPE.test(type)) {
+    throw new Error(`Invalid SQL type: ${type}`)
+  }
+}
+
 export class ERDModel {
   private tables: Map<string, Table> = new Map()
 
@@ -35,28 +42,30 @@ export class ERDModel {
     }
     const table: Table = { name, columns: [], foreignKeys: [] }
     this.tables.set(name, table)
-    return table
+    return structuredClone(table)
   }
 
   getTable(name: string): Table | undefined {
-    return this.tables.get(name)
+    const table = this.tables.get(name)
+    return table ? structuredClone(table) : undefined
   }
 
   getTables(): Table[] {
-    return Array.from(this.tables.values())
+    return Array.from(this.tables.values()).map(table => structuredClone(table))
   }
 
   addColumn(tableName: string, column: Column): void {
     assertSnakeCaseIdentifier('Table', tableName)
     assertSnakeCaseIdentifier('Column', column.name)
+    assertSafeSQLType(column.type)
     const table = this.tables.get(tableName)
     if (!table) {
       throw new Error(`Table '${tableName}' does not exist.`)
     }
     if (table.columns.some((c) => c.name === column.name)) {
       throw new Error(`Column '${column.name}' already exists in table '${tableName}'.`)
     }
-    table.columns.push(column)
+    table.columns.push(structuredClone(column))
   }
 
   addForeignKey(tableName: string, fk: ForeignKey): void {