Skip to content

Commit 0a9d7c2

Browse files
authored
security backport:#148 → 2.40 (#24156)
1 parent 4a29b4f commit 0a9d7c2

2 files changed

Lines changed: 42 additions & 1 deletion

File tree

dhis-2/dhis-services/dhis-service-administration/src/main/java/org/hisp/dhis/sqlview/DefaultSqlViewService.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import org.hisp.dhis.query.QueryUtils.PlaceholderQueryWithArgs;
6060
import org.hisp.dhis.security.acl.AclService;
6161
import org.hisp.dhis.system.grid.ListGrid;
62+
import org.hisp.dhis.system.util.SqlUtils;
6263
import org.hisp.dhis.user.CurrentUserService;
6364
import org.hisp.dhis.user.User;
6465
import org.springframework.stereotype.Service;
@@ -264,7 +265,7 @@ private OperatorWithPlaceHolderAndArg getFilterQuery(
264265
filter +=
265266
sqlHelper.whereAnd()
266267
+ " "
267-
+ columnName
268+
+ SqlUtils.quote(columnName)
268269
+ " "
269270
+ operatorWithPlaceHolderAndArg.getOperatorWithPlaceholder();
270271

dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/SqlViewControllerIntegrationTest.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import static org.hisp.dhis.web.HttpStatus.CREATED;
3232
import static org.hisp.dhis.web.HttpStatus.OK;
3333
import static org.junit.jupiter.api.Assertions.assertEquals;
34+
import static org.junit.jupiter.api.Assertions.assertFalse;
3435
import static org.junit.jupiter.api.Assertions.assertTrue;
3536

3637
import java.util.Set;
@@ -88,6 +89,45 @@ void sqlInjectionFieldTest() {
8889
.contains("Query failed because of a syntax error"));
8990
}
9091

92+
@Test
93+
void sqlInjectionFilterColumnNameUnionTest() {
94+
// The fixture views select 4 columns from optionvalue (uid, name, description,
95+
// sort_order). On vulnerable master a payload that smuggles a 4-column UNION
96+
// through the column-name slot returns userinfo.password (bcrypt) in the grid.
97+
// After the fix, SqlUtils.quote() turns the entire payload into a non-existent
98+
// identifier and Postgres rejects it. Either way, the response must not contain
99+
// a bcrypt-formatted hash.
100+
String injection = "1=1 UNION SELECT password, '', '', 0 FROM userinfo WHERE 'a'";
101+
HttpResponse response = GET(QUERY_PATH + "?filter=" + injection + ":neq:x");
102+
103+
String body = response.contentUnchecked().toString();
104+
assertFalse(
105+
body.toString().contains("$2a$"),
106+
"Filter column-name slot is injectable: response leaked a bcrypt hash. Body: " + body);
107+
}
108+
109+
@Test
110+
void sqlInjectionFilterColumnNameCastTest() {
111+
// Ahmed/M4xIq PoC (GHSA-pwmg-mvjw-4m23): smuggle a subquery via CAST(... AS int)
112+
// in the column-name slot, exfiltrate via the Postgres type-cast error message
113+
// ("invalid input syntax for type integer: \"<value>\"" embeds the subquery's
114+
// text result). After the fix, the entire CAST(...) expression becomes a quoted
115+
// identifier; Postgres errors out with "column does not exist" at parse time, so
116+
// the inner SELECT never executes and no value is leaked.
117+
String injection = "CAST((SELECT password FROM userinfo WHERE username='admin') AS int)";
118+
HttpResponse response = GET(QUERY_PATH + "?filter=" + injection + ":eq:1");
119+
120+
String body = response.contentUnchecked().toString();
121+
String s = body.toString();
122+
assertFalse(
123+
s.contains("$2a$"),
124+
"Filter column-name slot is injectable: response leaked a bcrypt hash. Body: " + body);
125+
assertFalse(
126+
s.toLowerCase().contains("invalid input syntax for type integer"),
127+
"Filter column-name slot is injectable: Postgres CAST error surfaces attacker subquery output. Body: "
128+
+ body);
129+
}
130+
91131
@Test
92132
void filterCriteriaFieldsQueryTest() {
93133
HttpResponse response =

0 commit comments

Comments
 (0)