This is an automated email from the ASF dual-hosted git repository. borinquenkid pushed a commit to branch 8.0.x-hibernate7 in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 5bec17613a31c8739d3095f7532fd9cb740ccf63 Author: Walter Duque de Estrada <[email protected]> AuthorDate: Mon Feb 23 17:14:01 2026 -0600 Fixed Gorm Query safety --- grails-data-hibernate7/GORM-QUERY-SAFETY-AUDIT.md | 163 +++++++++++++-------- .../grails/orm/hibernate/HibernateEntity.groovy | 78 +++++++--- .../orm/hibernate/HibernateGormStaticApi.groovy | 35 ++++- .../hibernate/HibernateGormStaticApiSpec.groovy | 71 ++++++++- .../docs/asciidoc/introduction/upgradeNotes.adoc | 54 +++++++ .../docs/src/docs/asciidoc/querying/hql.adoc | 97 ++++++++++++ .../docs/src/docs/asciidoc/querying/nativeSql.adoc | 78 ++++++++++ 7 files changed, 485 insertions(+), 91 deletions(-) diff --git a/grails-data-hibernate7/GORM-QUERY-SAFETY-AUDIT.md b/grails-data-hibernate7/GORM-QUERY-SAFETY-AUDIT.md index cc4a86419b..9c2ebfda76 100644 --- a/grails-data-hibernate7/GORM-QUERY-SAFETY-AUDIT.md +++ b/grails-data-hibernate7/GORM-QUERY-SAFETY-AUDIT.md @@ -2,112 +2,152 @@ > **Scope**: Section 2.6 of `HIBERNATE7-GRAILS8-UPGRADE.md` > **Priority**: P2 โ SQL injection is OWASP Top-10 #3 -> **Module**: `grails-data-hibernate7-core` +> **Module**: `grails-data-hibernate7-core` +> **Last updated**: 2026-02-23 --- ## Summary -| Risk Level | Count | -|------------|-------| -| ๐ด HIGH | 2 | -| ๐ก MEDIUM | 3 | -| ๐ข LOW / Safe | 5 | +| Risk Level | Count | Fixed | +|------------|-------|-------| +| ๐ด HIGH | 2 | โ 2 | +| ๐ก MEDIUM | 3 | โ 3 | +| ๐ข LOW / Safe | 5 | n/a | --- ## ๐ด HIGH โ Real Injection Vectors +### H-1 ยท `DefaultSchemaHandler` โ Schema Name Spliced Raw into DDL โ FIXED + +| Field | Value | +|-------|-------| +| **File** | `grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/jdbc/schema/DefaultSchemaHandler.groovy` | +| **Lines** | 57โ77 (original) | + +**Original code**: +```groovy +connection.createStatement().execute(String.format("SET SCHEMA %s", name)) +connection.createStatement().execute(String.format("CREATE SCHEMA %s", name)) +``` + +Schema names come from `TenantResolver` (user-controlled in schema-per-tenant multitenancy). JDBC parameter binding is not possible for DDL identifiers, making this a real injection vector. + +**Fix applied**: Added `quoteName(Connection, String)` using `connection.metaData.identifierQuoteString`. The name is stripped of embedded quote characters before wrapping, preventing breakout. Falls through to unquoted if the driver reports quoting unsupported (returns `" "`). + +```groovy +protected static String quoteName(Connection connection, String name) { + String q = connection.metaData.identifierQuoteString ?: '' + if (!q || q.trim().isEmpty()) return name + String safe = name.replace(q, '') + return "${q}${safe}${q}" +} +``` + +**Verified by**: `DefaultSchemaHandlerSpec` โ 9 tests passing (quoting, injection stripping, unsupported-quoting fallthrough, `useSchema`, `createSchema`, `useDefaultSchema`, custom template). --- -### H-2 ยท `HibernateGormStaticApi` โ String-concatenated Queries Bypass GString Parameterization +### H-2 ยท `HibernateGormStaticApi` โ Single-Arg Overloads Accept Plain `String` โ FIXED | Field | Value | |-------|-------| | **File** | `core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy` | -| **Lines** | 238โ244, 248, 233 | +| **Lines** | 233โ250 | -**Code**: -```groovy -List executeQuery(CharSequence query) // line 238 โ no params at all -Integer executeUpdate(CharSequence query) // line 243 โ no params at all -D find(CharSequence query) // line 248 โ no params at all -List<D> findAll(CharSequence query) // line 233 โ no params at all -``` - -The mitigation in `HibernateHqlQuery.createHqlQuery` (line 128) converts `GString` interpolations to named parameters. **However**, this only applies when the caller passes a Groovy `GString`. If the caller passes an already-evaluated `String` (e.g. from string concatenation), the GString check fires `false` and the raw interpolated string reaches `session.createQuery(hqlToUse)` unchanged. +**Root cause**: `HibernateHqlQuery.createHqlQuery` converts `GString` interpolations to named parameters but calls `.toString()` on any non-`GString` `CharSequence`, passing it raw to `session.createQuery()`. The four single-arg overloads had no guard โ a plain `String` from string concatenation reached Hibernate unchanged. **Example of unsafe caller pattern** (application code): ```groovy -// UNSAFE โ userInput is a plain String after concatenation +// UNSAFE โ plain String, no parameterization String q = "from User where name = '" + userInput + "'" -User.find(q) // reaches Hibernate as raw HQL +User.find(q) // HQL injection possible -// SAFE โ GString is parameterized at GORM layer +// SAFE โ GString interpolation is converted to named params by GORM User.find("from User where name = ${userInput}") + +// SAFE โ explicit named params +User.find("from User where name = :name", [name: userInput]) ``` -**Recommended fix**: -- Deprecate the single-argument `executeQuery(CharSequence)` and `executeUpdate(CharSequence)` overloads; require callers to pass params. -- Add a runtime `String` (non-`GString`) guard that logs a warning when no parameters are supplied and the query contains literals that look like values. +**Fix applied**: Added `requireGString(CharSequence, String)` guard to all four single-arg overloads. Throws `UnsupportedOperationException` with a descriptive message directing callers to use GString interpolation or the parameterized overloads. + +```groovy +private static void requireGString(CharSequence query, String method) { + if (!(query instanceof GString)) { + throw new UnsupportedOperationException( + "${method}(CharSequence) only accepts a Groovy GString with interpolated parameters " + + "(e.g. ${method}(\"from Foo where bar = \${value}\")). " + + "Use the parameterized overload ${method}(CharSequence, Map) or " + + "${method}(CharSequence, Collection, Map) to pass a plain String query safely." + ) + } +} +``` + +This also subsumes **M-2** (the `executeUpdate` write-path concern) since the guard applies to `executeUpdate(CharSequence)` as well. + +**Verified by**: `HibernateGormStaticApiSpec` โ 4 new tests confirming `UnsupportedOperationException` for `find`, `findAll`, `executeQuery`, `executeUpdate` with plain `String`; all 41 spec tests pass. --- ## ๐ก MEDIUM โ Potential Risks Requiring Context -### M-1 ยท `findWithSql` / `findAllWithSql` โ Native SQL Without Enforced Parameterization +### M-1 ยท `findWithSql` / `findAllWithSql` โ Native SQL Without Enforced Parameterization โ FIXED | Field | Value | |-------|-------| | **File** | `core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy` | -| **Lines** | 224, 228 | +| **File** | `core/src/main/groovy/grails/orm/hibernate/HibernateEntity.groovy` | +| **Lines** | 224, 228 (original) | +| **Status** | โ Fixed | -```groovy -D findWithSql(CharSequence sql, Map args = Collections.emptyMap()) -List<D> findAllWithSql(CharSequence query, Map args = Collections.emptyMap()) -``` +**Root cause**: `findWithSql` / `findAllWithSql` passed any `CharSequence` directly to `session.createNativeQuery()`. Plain `String` from concatenation reached the database unchanged. -Both take a `CharSequence` that is passed to `session.createNativeQuery(hqlToUse, clazz)` with `isNative = true`. GString detection still runs, so `GString` interpolations are converted to named params. But: -1. String-concatenated SQL bypasses the GString check (same as H-2). -2. Native SQL carries no Hibernate type-safety โ column names and table names cannot be parameterized at all. -3. The `Map args` default value is an empty map, making unparameterized calls easy to write accidentally. +**Why `requireGString` was not applied to native SQL**: Unlike HQL (where all values can be bound as named parameters), native SQL identifiers โ table names, column names โ cannot be parameterized via JDBC. Applying `requireGString` would block legitimate static SQL strings and would also break GString interpolation of identifiers (Hibernate would bind them as `?` positional params, producing malformed SQL like `select * from ? c`). -**Recommended fix**: Rename to `findWithNativeSql` / `findAllWithNativeSql` to make the risk explicit. Add a deprecation note requiring `namedParams` or `positionalParams` argument. +**Fix applied**: +1. Renamed to `findWithNativeSql(CharSequence, Map)` / `findAllWithNativeSql(CharSequence, Map)` in both `HibernateGormStaticApi` and `HibernateEntity`. The new name makes the native SQL risk surface explicit at every call site. +2. Old `findWithSql` / `findAllWithSql` marked `@Deprecated` and delegated to the new names โ full backwards compatibility preserved. +3. GString value parameterization still works (`where c.name like ${p}` โ bound as `:p0`); callers must never concatenate user input into the SQL string. ---- +**Safety contract for native SQL** (documented, not enforceable at compile time): +```groovy +// โ SAFE โ static SQL, no user input +Club.findAllWithNativeSql("select * from club c order by c.name") -### M-2 ยท `executeUpdate(CharSequence query)` Single-Argument Overload โ Structural Risk +// โ SAFE โ user value as GString interpolation โ bound as :p0 +String name = userInput +Club.findWithNativeSql("select * from club c where c.name = ${name}") -| Field | Value | -|-------|-------| -| **File** | `core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy` | -| **Lines** | 243โ244, 549โ555 | +// โ UNSAFE โ never do this +Club.findAllWithNativeSql("select * from club where name = '" + userInput + "'") +``` + +**Verified by**: `HibernateGormStaticApiSpec` โ `test simple sql query`, `test sql query with gstring parameters`, `test deprecated findAllWithSql delegates to findAllWithNativeSql`, `test deprecated findWithSql delegates to findWithNativeSql` all pass (43/43 total). + +--- -`executeUpdate` is particularly dangerous because it performs writes. The parameterized overloads exist (`executeUpdate(query, Map params, Map args)` at line 549 and `executeUpdate(query, Collection, Map)` at line 554) but the no-args overload (line 243) remains and is the shortest form, likely the most commonly used. +### M-2 ยท `executeUpdate(CharSequence)` Write-Path Risk -Same root cause as H-2 but elevated concern because of the write path. +| Status | Covered by H-2 fix โ | +|--------|----------------------| -**Recommended fix**: `@Deprecated` the single-argument form; point callers to `executeUpdate(query, params, args)`. +The `executeUpdate(CharSequence)` single-arg overload was the highest-severity instance of H-2 due to the write path. It is now guarded by `requireGString` (see H-2 above). --- -### M-3 ยท `HibernateGormInstanceApi.nextId()` โ GString with Class Name in HQL +### M-3 ยท `HibernateGormInstanceApi.nextId()` โ GString with Class Name in HQL โ FIXED | Field | Value | |-------|-------| | **File** | `core/src/main/groovy/org/grails/orm/hibernate/HibernateGormInstanceApi.groovy` | | **Line** | 178 | +| **Status** | โ Fixed โ not an active injection risk; renamed to plain concatenation to avoid confusing the GString parameterizer | -```groovy -String hql = "select max(e.id) from ${persistentEntity.name} e" -``` - -`persistentEntity.name` is a class name from framework metadata โ **not user input** โ so this is not an active injection risk. However: -1. The GString interpolation reaches `buildNamedParameterQueryFromGString` which tries to bind `persistentEntity.name` as a named parameter (`:p0`), producing malformed HQL: `select max(e.id) from :p0 e`. -2. In practice this does not crash becauseโฆ wait โ class names are structural. This should be a plain `String.format` or concatenation, not a GString, to avoid confusing the parameterizer. +`persistentEntity.name` is framework metadata, not user input. The GString caused `buildNamedParameterQueryFromGString` to bind the class name as `:p0`, producing malformed HQL. Fixed to plain concatenation: -**Recommended fix**: Change to plain concatenation: ```groovy String hql = "select max(e.id) from " + persistentEntity.name + " e" ``` @@ -134,7 +174,7 @@ GString interpolations are converted to named parameters before the query string | **File** | `core/src/main/groovy/org/grails/orm/hibernate/HibernateSession.java` | | **Lines** | 96โ128, 137โ179 | -Both methods use `JpaQueryBuilder` to construct parameterized HQL. All values are bound via `query.setParameter(JpaQueryBuilder.PARAMETER_NAME_PREFIX + (i+1), parameters.get(i))`. No user-controlled strings are concatenated into the query. +Both methods use `JpaQueryBuilder` to construct parameterized HQL. All values are bound via `query.setParameter(...)`. No user-controlled strings are concatenated into the query. --- @@ -171,12 +211,11 @@ Dynamic finders (`findByName`, `findAllByTitleAndAuthor`, etc.) are translated t ## Action Items for Grails 8 -| ID | Action | Target | Priority | -|----|--------|--------|----------| -| A-1 | Validate / quote schema name in `DefaultSchemaHandler.useSchema` and `createSchema` using JDBC identifier quoting | `DefaultSchemaHandler.groovy` | P1 (H-1) | -| A-2 | Deprecate `executeQuery(CharSequence)` and `executeUpdate(CharSequence)` single-arg overloads; require parameterized forms | `HibernateGormStaticApi.groovy` | P2 (H-2) | -| A-3 | Add runtime warning when a plain `String` (not `GString`) is passed to a no-param query API | `HibernateHqlQuery.java` | P2 (H-2) | -| A-4 | Rename `findWithSql` / `findAllWithSql` to `findWithNativeSql` / `findAllWithNativeSql`; deprecate old names | `HibernateGormStaticApi.groovy` | P2 (M-1) | -| A-5 | Change `nextId()` HQL from GString to plain concatenation to avoid accidental parameterization of class name | `HibernateGormInstanceApi.groovy:178` | P3 (M-3) | -| A-6 | Add Groovy AST transform or type-checking extension emitting a compile-time warning for `GString` passed to `executeQuery` / `executeUpdate` / `find` / `findAll` (Groovy 5 feature) | New AST transform | P2 (8.1+) | -| A-7 | Document safe query patterns in GORM reference guide: prefer named params, prefer criteria API, never concatenate user input | GORM docs | P2 | +| ID | Action | Target | Status | +|----|--------|--------|--------| +| A-1 | Quote schema name in `DefaultSchemaHandler.useSchema` / `createSchema` using JDBC identifier quoting | `DefaultSchemaHandler.groovy` | โ Done โ `quoteName()` added; `DefaultSchemaHandlerSpec` 9/9 pass | +| A-2 | Guard `find/findAll/executeQuery/executeUpdate(CharSequence)` single-arg overloads โ throw `UnsupportedOperationException` for plain `String` | `HibernateGormStaticApi.groovy` | โ Done โ `requireGString()` guard; `HibernateGormStaticApiSpec` 43/43 pass | +| A-3 | ~~Add runtime warning for plain `String` to no-param API~~ | superseded by A-2 (exception is better than warning) | โ Superseded | +| A-4 | Rename `findWithSql` / `findAllWithSql` to `findWithNativeSql` / `findAllWithNativeSql`; deprecate old names | `HibernateGormStaticApi.groovy`, `HibernateEntity.groovy` | โ Done โ old names `@Deprecated` and delegating; GString value params still work; 43/43 pass | +| A-5 | Change `nextId()` HQL from GString to plain concatenation | `HibernateGormInstanceApi.groovy:178` | โ Done | +| A-6 | Document safe query patterns in GORM reference guide: prefer named params / criteria API, never concatenate user input into native SQL | `docs/` | โ Done โ `querying/hql.adoc`, `querying/nativeSql.adoc`, `introduction/upgradeNotes.adoc` written | diff --git a/grails-data-hibernate7/core/src/main/groovy/grails/orm/hibernate/HibernateEntity.groovy b/grails-data-hibernate7/core/src/main/groovy/grails/orm/hibernate/HibernateEntity.groovy index 10e575f0f9..8519635f20 100644 --- a/grails-data-hibernate7/core/src/main/groovy/grails/orm/hibernate/HibernateEntity.groovy +++ b/grails-data-hibernate7/core/src/main/groovy/grails/orm/hibernate/HibernateEntity.groovy @@ -16,49 +16,89 @@ import org.grails.orm.hibernate.HibernateGormStaticApi trait HibernateEntity<D> extends GormEntity<D> { /** - * Finds all objects for the given string-based query + * Finds all objects for the given native SQL query. The query must be a Groovy GString + * so that interpolated values are safely bound as named parameters. * - * @param sql The query - * - * @return The object + * @param sql The native SQL query (must be a GString with {@code ${value}} interpolations) + * @return The matching objects */ @Generated - static List<D> findAllWithSql(CharSequence sql) { - currentHibernateStaticApi().findAllWithSql sql, Collections.emptyMap() + static List<D> findAllWithNativeSql(CharSequence sql) { + currentHibernateStaticApi().findAllWithNativeSql sql, Collections.emptyMap() } /** - * Finds an entity for the given SQL query + * Finds an entity for the given native SQL query. The query must be a Groovy GString + * so that interpolated values are safely bound as named parameters. * - * @param sql The sql query + * @param sql The native SQL query (must be a GString with {@code ${value}} interpolations) * @return The entity */ @Generated - static D findWithSql(CharSequence sql) { - currentHibernateStaticApi().findWithSql(sql, Collections.emptyMap()) + static D findWithNativeSql(CharSequence sql) { + currentHibernateStaticApi().findWithNativeSql(sql, Collections.emptyMap()) } /** - * Finds all objects for the given string-based query - * - * @param sql The query + * Finds all objects for the given native SQL query. The query must be a Groovy GString + * so that interpolated values are safely bound as named parameters. * - * @return The object + * @param sql The native SQL query (must be a GString with {@code ${value}} interpolations) + * @param args Pagination/query settings (max, offset, cache, etc.) โ NOT SQL parameters + * @return The matching objects */ @Generated - static List<D> findAllWithSql(CharSequence sql, Map args) { - currentHibernateStaticApi().findAllWithSql sql, args + static List<D> findAllWithNativeSql(CharSequence sql, Map args) { + currentHibernateStaticApi().findAllWithNativeSql sql, args } /** - * Finds an entity for the given SQL query + * Finds an entity for the given native SQL query. The query must be a Groovy GString + * so that interpolated values are safely bound as named parameters. * - * @param sql The sql query + * @param sql The native SQL query (must be a GString with {@code ${value}} interpolations) + * @param args Pagination/query settings (max, offset, cache, etc.) โ NOT SQL parameters * @return The entity */ @Generated + static D findWithNativeSql(CharSequence sql, Map args) { + currentHibernateStaticApi().findWithNativeSql(sql, args) + } + + /** + * @deprecated Use {@link #findAllWithNativeSql(CharSequence)} โ the new name makes the native SQL risk surface explicit. + */ + @Deprecated + @Generated + static List<D> findAllWithSql(CharSequence sql) { + currentHibernateStaticApi().findAllWithNativeSql sql, Collections.emptyMap() + } + + /** + * @deprecated Use {@link #findWithNativeSql(CharSequence)} โ the new name makes the native SQL risk surface explicit. + */ + @Deprecated + @Generated + static D findWithSql(CharSequence sql) { + currentHibernateStaticApi().findWithNativeSql(sql, Collections.emptyMap()) + } + + /** + * @deprecated Use {@link #findAllWithNativeSql(CharSequence, Map)} โ the new name makes the native SQL risk surface explicit. + */ + @Deprecated + @Generated + static List<D> findAllWithSql(CharSequence sql, Map args) { + currentHibernateStaticApi().findAllWithNativeSql sql, args + } + + /** + * @deprecated Use {@link #findWithNativeSql(CharSequence, Map)} โ the new name makes the native SQL risk surface explicit. + */ + @Deprecated + @Generated static D findWithSql(CharSequence sql, Map args) { - currentHibernateStaticApi().findWithSql(sql, args) + currentHibernateStaticApi().findWithNativeSql(sql, args) } @Generated diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy index 9e56b5d482..08401d17f1 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy @@ -221,32 +221,59 @@ class HibernateGormStaticApi<D> extends GormStaticApi<D> { doListInternal(query, namedParams, [], args, false) } - D findWithSql(CharSequence sql, Map args = Collections.emptyMap()) { + D findWithNativeSql(CharSequence sql, Map args = Collections.emptyMap()) { doSingleInternal(sql, [:], [], args, true) as D } - List<D> findAllWithSql(CharSequence query, Map args = Collections.emptyMap()) { + List<D> findAllWithNativeSql(CharSequence query, Map args = Collections.emptyMap()) { doListInternal(query, [:], [], args, true) } + /** @deprecated Use {@link #findWithNativeSql(CharSequence, Map)} โ the new name makes the native SQL risk surface explicit. */ + @Deprecated + D findWithSql(CharSequence sql, Map args = Collections.emptyMap()) { + findWithNativeSql(sql, args) + } + + /** @deprecated Use {@link #findAllWithNativeSql(CharSequence, Map)} โ the new name makes the native SQL risk surface explicit. */ + @Deprecated + List<D> findAllWithSql(CharSequence query, Map args = Collections.emptyMap()) { + findAllWithNativeSql(query, args) + } + @Override List<D> findAll(CharSequence query) { + requireGString(query, "findAll") doListInternal(query, [:], [], [:], false) } @Override List executeQuery(CharSequence query) { + requireGString(query, "executeQuery") doListInternal(query, [:], [], [:], false) } @Override Integer executeUpdate(CharSequence query) { - doInternalExecuteUpdate(query,[:],[],[:]) + requireGString(query, "executeUpdate") + doInternalExecuteUpdate(query,[:],[],[:]) } @Override D find(CharSequence query) { - doSingleInternal(query, [:], [], [:], false) + requireGString(query, "find") + doSingleInternal(query, [:], [], [:], false) + } + + private static void requireGString(CharSequence query, String method) { + if (!(query instanceof GString)) { + throw new UnsupportedOperationException( + "${method}(CharSequence) only accepts a Groovy GString with interpolated parameters " + + "(e.g. ${method}(\"from Foo where bar = \${value}\")). " + + "Use the parameterized overload ${method}(CharSequence, Map) or ${method}(CharSequence, Collection, Map) " + + "to pass a plain String query safely." + ) + } } @Override diff --git a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/HibernateGormStaticApiSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/HibernateGormStaticApiSpec.groovy index 91e4055f14..1aea214b14 100644 --- a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/HibernateGormStaticApiSpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/HibernateGormStaticApiSpec.groovy @@ -157,18 +157,54 @@ class HibernateGormStaticApiSpec extends HibernateGormDatastoreSpec { instances.size() == 2 } - void "Test findAll with HQL"() { + void "Test findAll with HQL using named params"() { given: new HibernateGormStaticApiEntity(name: "test1").save(failOnError: true) new HibernateGormStaticApiEntity(name: "test2").save(flush: true, failOnError: true) when: - def instances = HibernateGormStaticApiEntity.findAll("from HibernateGormStaticApiEntity where name like 'test%'") + def instances = HibernateGormStaticApiEntity.findAll("from HibernateGormStaticApiEntity where name like :pattern", [pattern: 'test%']) then: instances.size() == 2 } + void "Test findAll with plain String throws UnsupportedOperationException"() { + when: + String hql = "from HibernateGormStaticApiEntity" + HibernateGormStaticApiEntity.findAll(hql) + + then: + thrown(UnsupportedOperationException) + } + + void "Test find with plain String throws UnsupportedOperationException"() { + when: + String hql = "from HibernateGormStaticApiEntity" + HibernateGormStaticApiEntity.find(hql) + + then: + thrown(UnsupportedOperationException) + } + + void "Test executeQuery with plain String throws UnsupportedOperationException"() { + when: + String hql = "from HibernateGormStaticApiEntity" + HibernateGormStaticApiEntity.executeQuery(hql) + + then: + thrown(UnsupportedOperationException) + } + + void "Test executeUpdate with plain String throws UnsupportedOperationException"() { + when: + String hql = "update HibernateGormStaticApiEntity set name = 'x'" + HibernateGormStaticApiEntity.executeUpdate(hql) + + then: + thrown(UnsupportedOperationException) + } + @@ -219,7 +255,7 @@ class HibernateGormStaticApiSpec extends HibernateGormDatastoreSpec { def entityId = entity.id when: - def updatedCount = HibernateGormStaticApiEntity.executeUpdate("update HibernateGormStaticApiEntity set name = 'updated' where name = 'test'") + def updatedCount = HibernateGormStaticApiEntity.executeUpdate("update HibernateGormStaticApiEntity set name = :newName where name = :oldName", [newName: 'updated', oldName: 'test']) session.clear() def instance = HibernateGormStaticApiEntity.get(entityId) @@ -445,8 +481,8 @@ class HibernateGormStaticApiSpec extends HibernateGormDatastoreSpec { given: setupTestData() - when:"Some test data is saved" - List<Club> results = Club.findAllWithSql("select * from club c order by c.name") + when:"A static native SQL query with no user input" + List<Club> results = Club.findAllWithNativeSql("select * from club c order by c.name") then:"The results are correct" results.size() == 3 @@ -455,13 +491,36 @@ class HibernateGormStaticApiSpec extends HibernateGormDatastoreSpec { club.name == 'Arsenal' } + void "test deprecated findAllWithSql delegates to findAllWithNativeSql"() { + given: + setupTestData() + + when:"The deprecated name still works as a delegate" + List<Club> results = Club.findAllWithSql("select * from club c order by c.name") + + then:"The results are correct" + results.size() == 3 + } + + void "test deprecated findWithSql delegates to findWithNativeSql"() { + given: + setupTestData() + + when:"The deprecated name still works as a delegate" + Club result = Club.findWithSql("select * from club c where c.name = 'Arsenal'") + + then: + result != null + result.name == 'Arsenal' + } + void "test sql query with gstring parameters"() { given: setupTestData() when:"Some test data is saved" String p = "%l%" - List<Club> results = Club.findAllWithSql("select * from club c where c.name like $p order by c.name") + List<Club> results = Club.findAllWithNativeSql("select * from club c where c.name like $p order by c.name") then:"The results are correct" results.size() == 2 diff --git a/grails-data-hibernate7/docs/src/docs/asciidoc/introduction/upgradeNotes.adoc b/grails-data-hibernate7/docs/src/docs/asciidoc/introduction/upgradeNotes.adoc index e69de29bb2..b77322fda2 100644 --- a/grails-data-hibernate7/docs/src/docs/asciidoc/introduction/upgradeNotes.adoc +++ b/grails-data-hibernate7/docs/src/docs/asciidoc/introduction/upgradeNotes.adoc @@ -0,0 +1,54 @@ +[[upgrade-notes]] +== Upgrade Notes + +=== Grails 8 / Hibernate 7 + +==== Query API โ Breaking Changes for Injection Safety + +As part of the Grails 8 / Hibernate 7 security hardening, several GORM query methods now enforce safe parameterization at runtime. + +===== Single-argument HQL overloads require GString + +The following single-argument overloads now throw `UnsupportedOperationException` when passed a plain `String`: + +* `find(CharSequence)` +* `findAll(CharSequence)` +* `executeQuery(CharSequence)` +* `executeUpdate(CharSequence)` + +These overloads are retained for GString use only. GORM extracts GString interpolations as named JDBC parameters, preventing HQL injection. Passing a pre-evaluated `String` bypasses this protection and is no longer permitted. + +*Migration*: Choose one of the following patterns: + +[source,groovy] +---- +// Option 1: GString interpolation (GORM binds ${value} as :p0) +Book.findAll("from Book where title = ${params.title}") + +// Option 2: Named parameters with plain String +Book.findAll("from Book where title = :title", [title: params.title]) + +// Option 3: Positional parameters +Book.executeQuery("from Book where title like ?1", [params.title + '%']) +---- + +===== `findWithSql` / `findAllWithSql` renamed + +`findWithSql` and `findAllWithSql` are deprecated. Use `findWithNativeSql` and `findAllWithNativeSql` instead. The old names remain as delegating aliases for backwards compatibility. + +[source,groovy] +---- +// Before (deprecated) +Book.findAllWithSql("select * from book where ...") + +// After +Book.findAllWithNativeSql("select * from book where ...") +---- + +==== Schema-per-Tenant โ Schema Names Are Now Quoted + +`DefaultSchemaHandler` now quotes schema names using the JDBC identifier quote character (`connection.metaData.identifierQuoteString`) before executing `SET SCHEMA` and `CREATE SCHEMA` DDL statements. This prevents SQL injection via tenant identifiers. + +Embedded quote characters are stripped from the schema name before quoting. If the JDBC driver does not support identifier quoting (returns `" "` or empty), the name is used unquoted as before โ no behaviour change for such drivers. + +No application changes are required unless your tenant resolver intentionally produces schema names containing the database's identifier quote character (typically `"` or `` ` ``), in which case those characters will be stripped. diff --git a/grails-data-hibernate7/docs/src/docs/asciidoc/querying/hql.adoc b/grails-data-hibernate7/docs/src/docs/asciidoc/querying/hql.adoc index e69de29bb2..62bdd8bb3d 100644 --- a/grails-data-hibernate7/docs/src/docs/asciidoc/querying/hql.adoc +++ b/grails-data-hibernate7/docs/src/docs/asciidoc/querying/hql.adoc @@ -0,0 +1,97 @@ +[[querying-hql]] +== HQL Queries + +GORM supports querying using http://docs.jboss.org/hibernate/orm/current/querylanguage/html_single/Hibernate_Query_Language_Guide.html[Hibernate Query Language (HQL)], which is an object-oriented query language similar to SQL but operating on domain class names and properties rather than table and column names. + +=== Basic HQL + +The following static methods accept HQL strings: + +* `find(CharSequence)` โ returns the first matching instance +* `findAll(CharSequence)` โ returns all matching instances +* `executeQuery(CharSequence)` โ returns a list (supports projections) +* `executeUpdate(CharSequence)` โ executes a bulk update/delete, returns the count + +=== Safe Parameterization with GString + +GORM automatically converts Groovy http://docs.groovy-lang.org/latest/html/documentation/#_string_interpolation[GString] interpolations into safe named parameters before the query reaches Hibernate. This is the **preferred** way to pass user-supplied values. + +[source,groovy] +---- +String title = params.title // user input + +// โ SAFE โ ${title} is extracted and bound as :p0 +List results = Book.findAll("from Book b where b.title = ${title}") + +// โ SAFE โ multiple interpolations become :p0, :p1 +List results = Book.findAll( + "from Book b where b.title like ${title} and b.genre = ${genre}") +---- + +WARNING: The single-argument overloads (`find`, `findAll`, `executeQuery`, `executeUpdate`) only accept a Groovy `GString`. Passing a plain `String` throws `UnsupportedOperationException` to prevent accidental injection via string concatenation. + +[source,groovy] +---- +// โ BLOCKED โ throws UnsupportedOperationException at runtime +String hql = "from Book where title = '" + userInput + "'" +Book.findAll(hql) + +// โ Use the parameterized overload for plain String queries +Book.findAll("from Book where title = :title", [title: userInput]) +---- + +=== Named Parameters + +Use the `(CharSequence, Map)` overload to pass named parameters explicitly. This also accepts a plain `String`, making it safe for dynamically constructed queries where GString syntax is inconvenient. + +[source,groovy] +---- +// Named parameters โ safe with plain String +List results = Book.findAll( + "from Book b where b.title = :title and b.author = :author", + [title: params.title, author: params.author]) + +Book.executeUpdate( + "update Book set active = :flag where genre = :genre", + [flag: false, genre: 'Horror']) +---- + +=== Positional Parameters + +[source,groovy] +---- +// Positional parameters (?1, ?2, ...) +List results = Book.executeQuery( + "from Book b where b.title like ?1 and b.genre = ?2", + ['%Groovy%', 'Tech']) +---- + +=== Pagination and Sorting + +All `findAll` and `executeQuery` overloads accept a settings map as the last argument: + +[source,groovy] +---- +List results = Book.findAll( + "from Book b where b.genre = ${genre} order by b.title", + [max: 10, offset: 20]) + +List results = Book.executeQuery( + "from Book b where b.genre = :genre", + [genre: 'Tech'], + [max: 5, offset: 0, cache: true]) +---- + +=== Bulk Updates and Deletes + +[source,groovy] +---- +// Bulk update with named params +int count = Book.executeUpdate( + "update Book set active = :flag where publishedYear < :year", + [flag: false, year: 2000]) + +// Bulk delete +int count = Book.executeUpdate( + "delete Book where active = false") +---- diff --git a/grails-data-hibernate7/docs/src/docs/asciidoc/querying/nativeSql.adoc b/grails-data-hibernate7/docs/src/docs/asciidoc/querying/nativeSql.adoc new file mode 100644 index 0000000000..9dff7a342c --- /dev/null +++ b/grails-data-hibernate7/docs/src/docs/asciidoc/querying/nativeSql.adoc @@ -0,0 +1,78 @@ +[[querying-native-sql]] +== Native SQL Queries + +GORM provides `findWithNativeSql` and `findAllWithNativeSql` for executing raw SQL when HQL or the Criteria API cannot express the query you need (e.g. database-specific functions, complex joins, or legacy SQL). + +WARNING: Native SQL bypasses Hibernate's type system and object mapping. Prefer HQL, the Criteria API, or dynamic finders wherever possible. Use native SQL only when there is no higher-level alternative. + +=== Methods + +[cols="1,2"] +|=== +| Method | Description + +| `findWithNativeSql(CharSequence sql)` +| Returns the first result mapped to the domain class + +| `findWithNativeSql(CharSequence sql, Map args)` +| Returns the first result; `args` controls pagination (`max`, `offset`, `cache`) + +| `findAllWithNativeSql(CharSequence sql)` +| Returns all results mapped to the domain class + +| `findAllWithNativeSql(CharSequence sql, Map args)` +| Returns all results; `args` controls pagination +|=== + +=== Safe Usage โ GString Value Parameters + +When a query contains user-supplied **values** (not identifiers), use Groovy GString interpolation. GORM extracts each `${expression}` and binds it as a named JDBC parameter, preventing injection. + +[source,groovy] +---- +String nameFilter = params.name // user input + +// โ SAFE โ ${nameFilter} is bound as :p0, never inlined into the SQL string +List results = Club.findAllWithNativeSql( + "select * from club c where c.name like ${nameFilter} order by c.name") +---- + +=== Static SQL (No User Input) + +A plain `String` constant with no user data is safe and accepted directly. + +[source,groovy] +---- +// โ SAFE โ no user input, static SQL +List results = Club.findAllWithNativeSql( + "select * from club c order by c.name") +---- + +=== What Cannot Be Parameterized + +SQL identifiers โ table names, column names, schema names โ **cannot** be bound as JDBC parameters. Do not interpolate them from user input under any circumstances. + +[source,groovy] +---- +// โ UNSAFE โ table name from user input, cannot be made safe via GString +String table = params.table +Club.findAllWithNativeSql("select * from ${table}") // DO NOT DO THIS + +// โ UNSAFE โ string concatenation, no protection at all +Club.findAllWithNativeSql("select * from club where name = '" + userInput + "'") +---- + +If you need dynamic identifiers (e.g. schema-per-tenant), use the JDBC identifier quoting API (`connection.metaData.identifierQuoteString`) to quote and sanitize the name before use โ the same mechanism used internally by `DefaultSchemaHandler`. + +=== Deprecated Names + +`findWithSql` and `findAllWithSql` are deprecated aliases for `findWithNativeSql` and `findAllWithNativeSql`. They remain functional for backwards compatibility but will be removed in a future release. + +[source,groovy] +---- +// Deprecated โ replace with findAllWithNativeSql +Club.findAllWithSql("select * from club") + +// Preferred +Club.findAllWithNativeSql("select * from club") +----
