fix: add @ApiPermission to detailPlus and extend SQL injection blacklist#1166
fix: add @ApiPermission to detailPlus and extend SQL injection blacklist#1166jackieya wants to merge 1 commit intoPowerJob:masterfrom
Conversation
- Add @ApiPermission annotation to getInstanceDetailPlus method to enforce authentication - Add RUNSCRIPT, CALL, LINK, SCRIPT to SQL keyword blacklist to prevent H2 command injection Fixes Pre-Auth RCE via H2 SQL Injection in customQuery parameter
There was a problem hiding this comment.
Pull request overview
This PR addresses two security gaps in the /instance/detailPlus endpoint to prevent unauthorized access and reduce the risk of SQL-injection-based attacks against the worker’s task persistence query path.
Changes:
- Add
@ApiPermissionto/instance/detailPlusto enforce authentication/authorization (aligning with/instance/detail). - Extend the
customQuerySQL keyword blacklist with H2-specific dangerous commands (RUNSCRIPT,CALL,LINK,SCRIPT).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| String nonNullCustomQuery = Optional.ofNullable(customQuery).orElse(OmsConstant.NONE); | ||
| if (StringUtils.containsAnyIgnoreCase(nonNullCustomQuery, "delete", "update", "insert", "drop", "CREATE", "ALTER", "TRUNCATE", "RENAME", "LOCK", "GRANT", "REVOKE", "PREPARE", "EXECUTE", "COMMIT", "BEGIN")) { | ||
| if (StringUtils.containsAnyIgnoreCase(nonNullCustomQuery, "delete", "update", "insert", "drop", "CREATE", "ALTER", "TRUNCATE", "RENAME", "LOCK", "GRANT", "REVOKE", "PREPARE", "EXECUTE", "COMMIT", "BEGIN", "RUNSCRIPT", "CALL", "LINK", "SCRIPT")) { | ||
| throw new IllegalArgumentException("Don't get any ideas about the database, illegally query: " + customQuery); |
There was a problem hiding this comment.
The SQL-injection guard is still based on simple substring matching, which is easy to bypass with keyword obfuscation (e.g., inserting block comments or other separators inside a keyword) and it doesn't reject common statement separators/comment tokens (like ';', '--', '/* */'). For a security fix, consider switching to a stricter allowlist/pattern for customQuery (e.g., only permitting a limited WHERE-clause grammar) and explicitly rejecting statement separators and SQL comment syntaxes before passing the query to the worker DB layer.
| String customQuery = req.getCustomQuery(); | ||
| String nonNullCustomQuery = Optional.ofNullable(customQuery).orElse(OmsConstant.NONE); | ||
| if (StringUtils.containsAnyIgnoreCase(nonNullCustomQuery, "delete", "update", "insert", "drop", "CREATE", "ALTER", "TRUNCATE", "RENAME", "LOCK", "GRANT", "REVOKE", "PREPARE", "EXECUTE", "COMMIT", "BEGIN")) { | ||
| if (StringUtils.containsAnyIgnoreCase(nonNullCustomQuery, "delete", "update", "insert", "drop", "CREATE", "ALTER", "TRUNCATE", "RENAME", "LOCK", "GRANT", "REVOKE", "PREPARE", "EXECUTE", "COMMIT", "BEGIN", "RUNSCRIPT", "CALL", "LINK", "SCRIPT")) { |
There was a problem hiding this comment.
Adding very short blacklist tokens like "CALL"/"LINK"/"SCRIPT" combined with containsAnyIgnoreCase (substring match) can reject legitimate filter expressions where those sequences appear inside identifiers or string literals (e.g., a task_name LIKE '%callback%'). To avoid accidental breakage, consider matching whole SQL keywords (word boundaries / tokenization) rather than arbitrary substrings, or otherwise narrowing what customQuery is allowed to contain.
问题描述
/instance/detailPlus接口存在两个安全缺陷,组合利用可实现未授权远程代码执行(Pre-Auth RCE):@ApiPermission注解:getInstanceDetailPlus方法未添加权限校验注解,导致该接口无需认证即可访问customQuery参数 SQL 注入黑名单不完整:现有黑名单遗漏了 H2 数据库特有的危险命令(RUNSCRIPT、CALL、LINK、SCRIPT),攻击者可通过这些命令执行任意代码影响版本
PowerJob v5.1.0 ~ v5.1.2
修复内容
1. 强制鉴权(Patch A)
为
getInstanceDetailPlus方法添加@ApiPermission注解,与其他接口(如getInstanceDetail)保持一致:2. 扩展黑名单(Patch B)
将 H2 特有的危险关键字加入 customQuery 过滤黑名单: