Skip to content

Commit bf46014

Browse files
committed
ascanrules: SQLi PostgreSQL rename scan rule (all timing based)
Signed-off-by: kingthorin <kingthorin@users.noreply.github.com>
1 parent dc2e143 commit bf46014

File tree

4 files changed

+19
-14
lines changed

4 files changed

+19
-14
lines changed

addOns/ascanrules/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
77
### Changed
88
- Maintenance changes.
99
- Depends on an updated version of the Common Library add-on.
10+
- The SQL Injection - PostgreSQL scan rule and alerts have been renamed to clarify that they're timing based (Issue 7341).
1011

1112
### Added
1213
- Rules (as applicable) have been tagged in relation to HIPAA and PCI DSS.
Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@
4242
import org.zaproxy.zap.model.TechSet;
4343

4444
/**
45-
* The SqlInjectionPostgreScanRule identifies Postgresql specific SQL Injection vulnerabilities
46-
* using Postgresql specific syntax. If it doesn't use Postgresql specific syntax, it belongs in the
47-
* generic SQLInjection class! Note the ordering of checks, for efficiency is : 1) Error based (N/A)
48-
* 2) Boolean Based (N/A - uses standard syntax) 3) UNION based (N/A - uses standard syntax) 4)
49-
* Stacked (N/A - uses standard syntax) 5) Blind/Time Based (Yes)
45+
* The SqlInjectionPostgreSqlTimingScanRule identifies Postgresql specific SQL Injection
46+
* vulnerabilities using Postgresql specific syntax. If it doesn't use Postgresql specific syntax,
47+
* it belongs in the generic SQLInjection class! Note the ordering of checks, for efficiency is : 1)
48+
* Error based (N/A) 2) Boolean Based (N/A - uses standard syntax) 3) UNION based (N/A - uses
49+
* standard syntax) 4) Stacked (N/A - uses standard syntax) 5) Blind/Time Based (Yes)
5050
*
5151
* <p>See the following for some great specific tricks which could be integrated here
5252
* http://www.websec.ca/kb/sql_injection
@@ -60,7 +60,7 @@
6060
*
6161
* @author 70pointer
6262
*/
63-
public class SqlInjectionPostgreScanRule extends AbstractAppParamPlugin
63+
public class SqlInjectionPostgreSqlTimingScanRule extends AbstractAppParamPlugin
6464
implements CommonActiveScanRuleInfo {
6565

6666
private boolean doTimeBased = false;
@@ -198,7 +198,8 @@ public class SqlInjectionPostgreScanRule extends AbstractAppParamPlugin
198198
CommonAlertTag.OWASP_2017_A01_INJECTION,
199199
CommonAlertTag.WSTG_V42_INPV_05_SQLI,
200200
CommonAlertTag.HIPAA,
201-
CommonAlertTag.PCI_DSS));
201+
CommonAlertTag.PCI_DSS,
202+
CommonAlertTag.TEST_TIMING));
202203
alertTags.put(PolicyTag.DEV_FULL.getTag(), "");
203204
alertTags.put(PolicyTag.QA_STD.getTag(), "");
204205
alertTags.put(PolicyTag.QA_FULL.getTag(), "");
@@ -208,7 +209,8 @@ public class SqlInjectionPostgreScanRule extends AbstractAppParamPlugin
208209
}
209210

210211
/** for logging. */
211-
private static final Logger LOGGER = LogManager.getLogger(SqlInjectionPostgreScanRule.class);
212+
private static final Logger LOGGER =
213+
LogManager.getLogger(SqlInjectionPostgreSqlTimingScanRule.class);
212214

213215
@Override
214216
public int getId() {

addOns/ascanrules/src/main/resources/org/zaproxy/zap/extension/ascanrules/resources/Messages.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ ascanrules.sqlinjection.mssql.name = SQL Injection - MsSQL
186186
ascanrules.sqlinjection.mysql.name = SQL Injection - MySQL
187187
ascanrules.sqlinjection.name = SQL Injection
188188
ascanrules.sqlinjection.oracle.name = SQL Injection - Oracle
189-
ascanrules.sqlinjection.postgres.name = SQL Injection - PostgreSQL
189+
ascanrules.sqlinjection.postgres.name = SQL Injection - PostgreSQL (Time Based)
190190
ascanrules.sqlinjection.refs = https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html
191191
ascanrules.sqlinjection.soln = Do not trust client side input, even if there is client side validation in place.\nIn general, type check all data on the server side.\nIf the application uses JDBC, use PreparedStatement or CallableStatement, with parameters passed by '?'\nIf the application uses ASP, use ADO Command Objects with strong type checking and parameterized queries.\nIf database Stored Procedures can be used, use them.\nDo *not* concatenate strings into queries in the stored procedure, or use 'exec', 'exec immediate', or equivalent functionality!\nDo not create dynamic SQL queries using simple string concatenation.\nEscape all data received from the client.\nApply an 'allow list' of allowed characters, or a 'deny list' of disallowed characters in user input.\nApply the principle of least privilege by using the least privileged database user possible.\nIn particular, avoid using the 'sa' or 'db-owner' database users. This does not eliminate SQL injection, but minimizes its impact.\nGrant the minimum database access that is necessary for the application.
192192
ascanrules.sqlinjection.sqlite.alert.errorbased.extrainfo = The following known SQLite error message was provoked: [{0}].
Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,13 @@
4141
import org.zaproxy.zap.model.TechSet;
4242
import org.zaproxy.zap.testutils.NanoServerHandler;
4343

44-
/** Unit test for {@link SqlInjectionPostgreScanRule}. */
45-
class SqlInjectionPostgreScanRuleUnitTest extends ActiveScannerTest<SqlInjectionPostgreScanRule> {
44+
/** Unit test for {@link SqlInjectionPostgreSqlTimingScanRule}. */
45+
class SqlInjectionPostgreSqlTimingScanRuleUnitTest
46+
extends ActiveScannerTest<SqlInjectionPostgreSqlTimingScanRule> {
4647

4748
@Override
48-
protected SqlInjectionPostgreScanRule createScanner() {
49-
return new SqlInjectionPostgreScanRule();
49+
protected SqlInjectionPostgreSqlTimingScanRule createScanner() {
50+
return new SqlInjectionPostgreSqlTimingScanRule();
5051
}
5152

5253
@Test
@@ -158,7 +159,7 @@ void shouldReturnExpectedMappings() {
158159
// Then
159160
assertThat(cwe, is(equalTo(89)));
160161
assertThat(wasc, is(equalTo(19)));
161-
assertThat(tags.size(), is(equalTo(10)));
162+
assertThat(tags.size(), is(equalTo(11)));
162163
assertThat(
163164
tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()),
164165
is(equalTo(true)));
@@ -169,6 +170,7 @@ void shouldReturnExpectedMappings() {
169170
tags.containsKey(CommonAlertTag.WSTG_V42_INPV_05_SQLI.getTag()), is(equalTo(true)));
170171
assertThat(tags.containsKey(CommonAlertTag.HIPAA.getTag()), is(equalTo(true)));
171172
assertThat(tags.containsKey(CommonAlertTag.PCI_DSS.getTag()), is(equalTo(true)));
173+
assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true)));
172174
assertThat(tags.containsKey(PolicyTag.DEV_FULL.getTag()), is(equalTo(true)));
173175
assertThat(tags.containsKey(PolicyTag.QA_STD.getTag()), is(equalTo(true)));
174176
assertThat(tags.containsKey(PolicyTag.QA_FULL.getTag()), is(equalTo(true)));

0 commit comments

Comments
 (0)