Security: phase 2 SQL Injection remediation#2400
Merged
LiamStanziani merged 17 commits intomaintenancefrom Apr 15, 2026
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @LiamStanziani, your pull request is larger than the review limit of 150000 diff characters
📝 WalkthroughWalkthroughThis PR implements comprehensive SQL injection prevention by systematically converting string-concatenated SQL queries to parameterized prepared statements across the entire codebase. It introduces new utility classes for SQL validation, updates over 150 files including DAOs, business logic, form handlers, utilities, and JSP pages with parameterized query execution, and adds validation for user-controlled identifiers and SQL parameters. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes This is a large-scale, high-complexity refactoring affecting 150+ files with heterogeneous changes (DAOs, business logic, JSP, utilities, new APIs). Multiple patterns applied (parameterized queries, validation logic, API deprecations, new helper methods) across diverse modules require careful verification of parameter binding correctness, deprecated method migration, and potential behavioral changes in dynamic query construction. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
LiamStanziani
changed the title
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
LiamStanziani
changed the title
LiamStanziani
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements extensive SQL injection mitigations by replacing raw SQL concatenation with parameterized queries and introducing a PreparedSQL utility. It also adds regex-based whitelisting for dynamic identifiers like table and column names. Feedback highlights several critical resource management issues where database objects are not closed, as well as thread-safety concerns and logic bugs in PostgreSQL-specific code and report template processing.
LiamStanziani
changed the title
There was a problem hiding this comment.
Pull request overview
This PR continues the “phase 2” SQL-injection remediation work by replacing string-concatenated SQL with parameterized queries and by adding allowlist validation where dynamic SQL fragments (e.g., ORDER BY) are unavoidable.
Changes:
- Converted multiple JSP SQL statements from string concatenation to prepared/parameterized execution.
- Added allowlist validation for dynamic ordering/column selection inputs (e.g.,
orderby, lookup table IDs). - Introduced/extended reusable parameterized-SQL helpers (
PreparedSQL) and added stricter validation for dynamic table usage inJDBCUtil.
Reviewed changes
Copilot reviewed 149 out of 149 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/webapp/report/reportResult.jsp | Validates id as numeric before use to reduce injection risk. |
| src/main/webapp/report/reportbilledvisit1.jsp | Converts date/provider query fragments to parameterized queries. |
| src/main/webapp/oscarMDS/PatientSearch.jsp | Adds orderby allowlist to prevent injection via ORDER BY. |
| src/main/webapp/eform/efmformapconfig_lookup.jsp | Switches eForm AP SQL substitution to parameterized execution via parameterizeFields. |
| src/main/webapp/decision/antenatal/antenatalplannerprint.jsp | Uses prepared query with bound ID parameter. |
| src/main/webapp/decision/antenatal/antenatalplanner.jsp | Uses prepared query with bound ID parameter. |
| src/main/webapp/billing/CA/BC/onSearch3rdBillAddr.jsp | Adds allowlists + parameter binding for search and ordering. |
| src/main/webapp/admin/securitysearchresults.jsp | Replaces dynamic order-by DAO call with a fixed safe DAO method. |
| src/main/webapp/admin/logReport.jsp | Parameterizes provider/site filtering and content filter in log queries. |
| src/main/java/ca/openosp/openo/www/lookup/LookupList2Action.java | Adds tableId validation for defense-in-depth on lookup routing. |
| src/main/java/ca/openosp/openo/www/lookup/LookupCodeList2Action.java | Adds tableId validation for lookup list retrieval. |
| src/main/java/ca/openosp/openo/www/lookup/LookupCodeEdit2Action.java | Adds validation for lookup tableId/code identifiers. |
| src/main/java/ca/openosp/openo/util/PreparedSQL.java | Introduces a value object for SQL text + ordered bind params. |
| src/main/java/ca/openosp/openo/util/JDBCUtil.java | Validates dynamic table names and uses prepared statements for zip-import DB operations. |
Comments suppressed due to low confidence (1)
src/main/webapp/oscarMDS/PatientSearch.jsp:231
limit1/limit2are taken directly from the request and used to build alimit ... offset ...SQL fragment without validation, but that fragment is currently unused (the+ " " + limitpart is commented out). To avoid future accidental reintroduction of an injection vector and reduce dead code, either remove these limit variables entirely or validate them as integers and apply paging via a safe API (e.g.,setFirstResult/setMaxResultsor parameterized LIMIT/OFFSET where supported).
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (18)
src/main/webapp/admin/securitysearchresults.jsp (1)
150-159:⚠️ Potential issue | 🟡 MinorPotential NullPointerException if query parameters are missing.
If
keywordorsearch_moderequest parameters are null, this code will throw NPE at lines 151 and 154. Consider adding null checks.🛡️ Proposed null-safety fix
//if action is good, then give me the result String searchMode = request.getParameter("search_mode"); - String keyword = request.getParameter("keyword").trim() + "%"; + String keywordParam = request.getParameter("keyword"); + String keyword = (keywordParam != null ? keywordParam.trim() : "") + "%"; // if search mode is provider_no - if (searchMode.equals("search_providerno")) + if ("search_providerno".equals(searchMode)) securityList = securityDao.findByLikeProviderNo(keyword); // if search mode is user_name - if (searchMode.equals("search_username")) + if ("search_username".equals(searchMode)) securityList = securityDao.findByLikeUserName(keyword);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/admin/securitysearchresults.jsp` around lines 150 - 159, The code reads request parameters into searchMode and keyword and calls keyword.trim() and searchMode.equals(...) which can throw NullPointerException; update the handling in this block so you null-check both request.getParameter("search_mode") and request.getParameter("keyword") before using them, provide sensible defaults (e.g., empty string) or early-return when missing, and only call keyword.trim() + "%" when keyword is non-null/non-empty; then branch on searchMode safely (use "searchMode != null && searchMode.equals(...)" or switch on a normalized value) before invoking securityDao.findByLikeProviderNo or findByLikeUserName so those DAO methods are only called with a valid keyword.database/mysql/importCasemgmt.java (1)
71-72:⚠️ Potential issue | 🔴 CriticalCritical: Remove database password from log output.
Logging credentials to stdout is a severe security vulnerability. This exposes sensitive authentication data in logs, terminals, and potentially log aggregation systems.
Proposed fix
- String passwd = prop.getProperty("db_password"); - System.out.println("DB PASSWD " + passwd); + String passwd = prop.getProperty("db_password"); + // Password intentionally not logged for security🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@database/mysql/importCasemgmt.java` around lines 71 - 72, The code is printing the database password to stdout via System.out.println("DB PASSWD " + passwd); — remove that line and any other logging of the sensitive variable passwd (obtained from prop.getProperty("db_password")). Replace it with a non-sensitive log (e.g., an informational message that the DB password was loaded or omit logging entirely) or use a secure logger without including secrets; ensure functions/classes referencing passwd (the variable in importCasemgmt.java) do not expose it in logs or exceptions.src/main/java/ca/openosp/openo/commn/dao/SiteDaoImpl.java (1)
242-249:⚠️ Potential issue | 🔴 CriticalCritical bug: Named parameter placeholders with positional binding will fail at runtime.
The query uses named parameters (
:groupno,:sitename) but thesetParametercalls use positional indices (1, 2). This mismatch will throw anIllegalArgumentExceptionat runtime.Since this PR is addressing parameterization across DAOs, this should be fixed for consistency.
🐛 Proposed fix: Use indexed positional parameters
`@Override` public Long site_searchmygroupcount(String myGroupNo, String siteName) { - Query query = entityManager.createNativeQuery("select count(provider_no) from mygroup where mygroup_no=:groupno and provider_no in (select ps.provider_no from providersite ps inner join site s on ps.site_id = s.site_id where s.name = :sitename)"); - query.setParameter(1, myGroupNo); - query.setParameter(2, siteName); + Query query = entityManager.createNativeQuery("select count(provider_no) from mygroup where mygroup_no = ?1 and provider_no in (select ps.provider_no from providersite ps inner join site s on ps.site_id = s.site_id where s.name = ?2)"); + query.setParameter(1, myGroupNo); + query.setParameter(2, siteName); Long result = ((BigInteger) query.getSingleResult()).longValue(); return result; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/commn/dao/SiteDaoImpl.java` around lines 242 - 249, The method site_searchmygroupcount uses a native query with named parameters (:groupno, :sitename) but binds them positionally; update the parameter binding in site_searchmygroupcount to use the named parameter API (call query.setParameter("groupno", myGroupNo) and query.setParameter("sitename", siteName)) so the names used in the SQL match the names used when setting parameters on the Query returned by entityManager.createNativeQuery.src/main/java/ca/openosp/openo/commn/dao/DxresearchDAOImpl.java (1)
399-419:⚠️ Potential issue | 🟡 MinorEnum validation pattern is correct, but sanitize the log output.
The approach of validating
codingSystemagainst an enum before using it in dynamic table/column names is the correct pattern for this use case (parameterized queries cannot be used for identifiers). The actual data values are properly parameterized.However, line 406 logs the invalid
codingSystemvalue directly without sanitization. WhilecodingSystemis unlikely to contain PHI, defensive coding requires sanitizing all user input before logging.🛡️ Proposed fix to sanitize log output
- logger.error("Invalid coding system provided: " + codingSystem); + logger.error("Invalid coding system provided: " + org.owasp.encoder.Encode.forJava(codingSystem));Based on coding guidelines: "Sanitize user input for logging using Encode.forJava() to prevent log injection"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/commn/dao/DxresearchDAOImpl.java` around lines 399 - 419, In findResearchAndCodingSystemByDemographicAndCondingSystem validate codingSystem as you already do with AbstractCodeSystemDaoImpl.codingSystem.valueOf(...), but when logging the invalid value use a sanitized form (e.g., Encode.forJava(codingSystem)) instead of logging raw input; update the logger.error call to include the encoded/sanitized codingSystem (or a minimal safe message) so you avoid possible log injection while keeping the same validation flow.src/main/java/ca/openosp/openo/www/lookup/LookupList2Action.java (1)
66-76:⚠️ Potential issue | 🟠 MajorThis action still executes lookup reads without an authorization gate.
Both
list()andsearch()now validatetableId, but neither path performsSecurityInfoManager.hasPrivilege()before loading lookup data. Please enforce the privilege check at the start ofexecute()before dispatching.As per coding guidelines, "ALL actions MUST include SecurityInfoManager.hasPrivilege() checks before performing operations" and "Security check must be the FIRST operation in action methods - before any business logic".
Also applies to: 91-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/www/lookup/LookupList2Action.java` around lines 66 - 76, Add a SecurityInfoManager.hasPrivilege() check as the very first operation in the execute() method (before any request parameter reads, validateTableId calls, or dispatch to search()/list()), and return or throw appropriately when the privilege is missing; ensure the same first-operation security gating covers both flows dispatched to list() and search() (and likewise ensure any duplicate entry points that load lookup data such as the code referenced around the search/list calls also perform the same check). Use the SecurityInfoManager.hasPrivilege(...) call and place it at the top of execute() so no business logic (including request.getParameter or validateTableId in list()/search()) runs without the privilege check.src/main/java/ca/openosp/openo/messenger/docxfer/send/MsgGenerate.java (1)
235-247:⚠️ Potential issue | 🔴 CriticalSQL injection vulnerability: Config attributes must not be directly concatenated into SQL queries.
The
sqlWhereandsqlOrderattributes from the XML configuration are appended directly to the SQL string (lines 241-246), violating the parameterized query requirement. Even ifDocXferConfig.xmlis protected on the server, config files can be compromised and should never be trusted as safe for SQL concatenation. These attributes must be validated and either:
- Excluded from the query entirely, or
- Properly parameterized (added as bind parameters), or
- Validated against a whitelist of allowed column/clause patterns
The coding guidelines require: "Use parameterized queries ONLY - never use string concatenation in SQL queries."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/messenger/docxfer/send/MsgGenerate.java` around lines 235 - 247, The code in MsgGenerate that appends cfgTable.getAttribute("sqlWhere") and cfgTable.getAttribute("sqlOrder") directly to the sql String creates an SQL injection risk; instead, change the logic that builds the sql variable so that sqlWhere is not concatenated raw — either remove it or parse and validate it against a strict whitelist of allowed column names/operators and translate each allowed filter into bindable predicates with PreparedStatement parameters (use setX on the PreparedStatement created after building the parameterized WHERE clause), and for sqlOrder, only allow a validated whitelist of column names and an optional direction (ASC/DESC) and then append the validated ORDER BY fragment; update code references around cfgTable, getAttribute("sqlWhere"), getAttribute("sqlOrder"), and the PreparedStatement creation in MsgGenerate to use the parameterized/validated approach rather than raw concatenation.src/main/java/ca/openosp/openo/www/lookup/LookupCodeEdit2Action.java (1)
50-55:⚠️ Potential issue | 🟠 MajorMissing
SecurityInfoManager.hasPrivilege()check before business logic.Per coding guidelines, all actions must include
SecurityInfoManager.hasPrivilege()checks before performing operations. Theexecute()method proceeds directly tosave()orloadCode()without verifying user privileges.🛡️ Suggested fix
+ private SecurityInfoManager securityInfoManager = SpringUtils.getBean(SecurityInfoManager.class); + public String execute() throws Exception { + if (!securityInfoManager.hasPrivilege(LoggedInInfo.getLoggedInInfoFromSession(request), "_admin", "r", null)) { + throw new SecurityException("Missing required privilege for lookup management"); + } if ("save".equals(request.getParameter("method"))) { return save(); } return loadCode(); }As per coding guidelines: "ALL actions MUST include SecurityInfoManager.hasPrivilege() checks before performing operations" and "Security check must be the FIRST operation in action methods - before any business logic".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/www/lookup/LookupCodeEdit2Action.java` around lines 50 - 55, In LookupCodeEdit2Action.update the execute() method to call SecurityInfoManager.hasPrivilege(...) as the very first operation (before any business logic) and short-circuit if the check fails; specifically, add the privilege check at the top of execute() and only proceed to call save() or loadCode() when hasPrivilege() returns true, returning the appropriate no-privilege result (or handling per app convention) when it returns false.src/main/java/ca/openosp/openo/commn/dao/PregnancyFormsDao.java (1)
13-27:⚠️ Potential issue | 🟡 MinorSQL injection fix is correct; resource leak in exception path.
The parameterized query correctly prevents SQL injection. However, if
rs.next()orrs.getInt("id")throws an exception that isn'tSQLException, or if aSQLExceptionoccurs afterrsis obtained, theResultSetwon't be closed. Thecatchblock at line 23-25 returns without closingrs.🛡️ Suggested fix using try-with-resources
public static Integer getLatestFormIdByPregnancy(Integer episodeId) { String sql = "SELECT id from formONAREnhancedRecord WHERE episodeId = ? ORDER BY formEdited DESC"; - try { - ResultSet rs = DBHandler.GetPreSQL(sql, episodeId); + try (ResultSet rs = DBHandler.GetPreSQL(sql, episodeId)) { if (rs.next()) { Integer id = rs.getInt("id"); - rs.close(); return id; } - rs.close(); } catch (SQLException e) { MiscUtils.getLogger().error("Error", e); return 0; } return 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/commn/dao/PregnancyFormsDao.java` around lines 13 - 27, In getLatestFormIdByPregnancy, the ResultSet obtained from DBHandler.GetPreSQL(sql, episodeId) may not be closed if an exception occurs; change the method to acquire the ResultSet in a try-with-resources (e.g., try (ResultSet rs = DBHandler.GetPreSQL(...)) { ... }) so rs is always closed, move rs.next()/rs.getInt("id") inside that try block, and keep the existing SQLException catch that returns 0; reference: getLatestFormIdByPregnancy and DBHandler.GetPreSQL.src/main/webapp/report/reportonbilledphcp.jsp (2)
137-141:⚠️ Potential issue | 🟠 MajorMissing OWASP encoding for user-controlled output creates XSS vulnerabilities.
Multiple user-controlled values are output without encoding:
- Lines 137, 140:
startDate,endDatein form inputs- Line 676:
providerNamein table header- Line 686:
startDate,endDatein display text- Lines 772-774:
vServiceCode.get(i),vServiceDesc.get(i)from databaseFor a security-focused PR, these should use OWASP Encoder.
🛡️ Proposed fix for XSS protection
Add the import at the top of the file:
<%@ page import="org.owasp.encoder.Encode" %>Then encode outputs:
-<input type="text" name="startDate" id="startDate" value="<%=startDate!=null?startDate:""%>" size="10" +<input type="text" name="startDate" id="startDate" value="<%=startDate!=null?Encode.forHtmlAttribute(startDate):""%>" size="10" -<input type="text" name="endDate" id="endDate" value="<%=endDate!=null?endDate:""%>" size="10" +<input type="text" name="endDate" id="endDate" value="<%=endDate!=null?Encode.forHtmlAttribute(endDate):""%>" size="10" -<th align="left"><font face="Helvetica" color="white"><%=providerName%> +<th align="left"><font face="Helvetica" color="white"><%=Encode.forHtml(providerName)%> -<td>Period: ( <%= startDate %> ~ <%= endDate %> )</td> +<td>Period: ( <%= Encode.forHtml(startDate) %> ~ <%= Encode.forHtml(endDate) %> )</td> -<td><%=vServiceCode.get(i)%></td> -<td><%=vServiceDesc.get(i)%></td> +<td><%=Encode.forHtml((String)vServiceCode.get(i))%></td> +<td><%=Encode.forHtml((String)vServiceDesc.get(i))%></td>As per coding guidelines: "Use Encode.forHtml(), Encode.forHtmlAttribute()... from OWASP Encoder for ALL user inputs in web output"
Also applies to: 676-686, 772-774
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/report/reportonbilledphcp.jsp` around lines 137 - 141, The JSP outputs several user-controlled values without encoding (startDate, endDate input values and display text, providerName table header, and vServiceCode.get(i)/vServiceDesc.get(i) list entries), creating XSS risk; add the OWASP Encoder import (org.owasp.encoder.Encode) at the top of the file and replace raw outputs: use Encode.forHtmlAttribute(...) for values rendered inside attributes (e.g., the value attributes of startDate and endDate inputs) and Encode.forHtml(...) for values rendered as HTML content (e.g., providerName header, the displayed startDate/endDate text, and the vServiceCode/vServiceDesc outputs) so all user-controlled data is safely encoded before rendering.
125-126:⚠️ Potential issue | 🟡 MinorMissing CSRF token in POST form.
The form uses
method="POST"but lacks CSRF protection.🛡️ Proposed fix
<form name="myform" action="reportonbilledphcp.jsp" method="POST" onsubmit="return onSub();"> + <input type="hidden" name="${_csrf.parameterName}" value="${_csrf.token}"/> <table width="100%" border="0" bgcolor="ivory" cellspacing="1"As per coding guidelines: "Include CSRF token in all HTML forms"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/report/reportonbilledphcp.jsp` around lines 125 - 126, The POST form named "myform" (onsubmit handler onSub()) lacks a CSRF token; add a hidden input containing the server-generated CSRF value (e.g. from request attribute/session like csrfToken or ${csrfToken}) inside the form and ensure the server-side handler that processes reportonbilledphcp.jsp validates that token; update any client-side code in onSub() if it inspects form fields to not strip the hidden input and ensure token generation/placement uses the same symbol your server exposes (e.g. request.getAttribute("csrfToken") or session.getAttribute("CSRF_TOKEN")).src/main/webapp/eform/efmformapconfig_lookup.jsp (1)
39-53:⚠️ Potential issue | 🟠 MajorImport OWASP Encoder and use it at all HTML attribute sinks.
This file has multiple XSS encoding violations:
- Line 16:
request.getParameter("oscarAPCacheLookupType")rendered directly in attribute value without encoding- Line 51: Uses
StringEscapeUtils.escapeHtml4()instead of OWASP Encoder (non-compliant with repo guidelines)- Lines 53, 57, 61:
keyvariable fromrequest.getParameterValues()rendered directly in thenameattribute without encodingAdd
<%@ page import="org.owasp.encoder.Encode" %>and replace:
- Line 16 attribute value with
Encode.forHtmlAttribute(request.getParameter("oscarAPCacheLookupType"))- Line 51 encoding with
Encode.forHtml(values.get(i))- Lines 53, 57, 61: both
nameandvalueattributes withEncode.forHtmlAttribute(key)andEncode.forHtmlAttribute(output)Per coding guidelines: "Use OWASP Encoder library from org.owasp.encoder package - never use custom encoding" and "
**/*.jsp: Use Encode.forHtmlAttribute() ... for ALL user inputs in web output".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/eform/efmformapconfig_lookup.jsp` around lines 39 - 53, Import org.owasp.encoder.Encode in the JSP and replace all current HTML sinks to use OWASP Encoder: change any direct rendering of request parameters (e.g. request.getParameter("oscarAPCacheLookupType")) to Encode.forHtmlAttribute(...), replace the use of org.apache.commons.text.StringEscapeUtils.escapeHtml4(values.get(i)) with Encode.forHtml(values.get(i)), and ensure the attributes that output the key and output variables in the input tag (the name and value attributes that use key and output) use Encode.forHtmlAttribute(key) and Encode.forHtmlAttribute(output); keep the existing logic that computes names, values, sql, swp and uses DatabaseAP.parserReplace/EFormUtil.getValues but pass encoded strings to the rendered HTML.src/main/webapp/report/reportbilledvisit2.jsp (3)
225-228:⚠️ Potential issue | 🟡 MinorXSS vulnerability: Nurse names output without encoding.
vNurse.get(i)values from database are output without HTML encoding.🛡️ Proposed fix
- <TH width="6%"><%="pat_" + vNurse.get(i)%> + <TH width="6%"><%="pat_" + Encode.forHtml((String)vNurse.get(i))%> </TH> - <TH width="6%"><%="vis_" + vNurse.get(i)%> + <TH width="6%"><%="vis_" + Encode.forHtml((String)vNurse.get(i))%> </TH>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/report/reportbilledvisit2.jsp` around lines 225 - 228, The nurse names are written directly via vNurse.get(i) in reportbilledvisit2.jsp (<TH> elements) causing an XSS risk; replace the raw scriptlet output with an HTML-encoded value (e.g., use JSTL/EL <c:out value="..."/> or fn:escapeXml, or call a server-side encoder such as StringEscapeUtils.escapeHtml4) when rendering vNurse.get(i) and any other user-sourced values so special characters are escaped before writing to the TH elements.
206-206:⚠️ Potential issue | 🟠 MajorXSS vulnerability:
sdateandedateoutput without encoding.Request parameters are output directly in HTML without OWASP encoding. While SQL injection is fixed, this introduces XSS risk.
As per coding guidelines: "Use Encode.forHtml()... for ALL user inputs in web output."
🛡️ Proposed fix
Add OWASP Encoder import at the top:
<%@ page import="org.owasp.encoder.Encode" %>Then encode the output:
- <td>Period: (<%=sdate%> ~ <%=edate%>)</td> + <td>Period: (<%=Encode.forHtml(sdate)%> ~ <%=Encode.forHtml(edate)%>)</td>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/report/reportbilledvisit2.jsp` at line 206, Outputting sdate and edate directly in reportbilledvisit2.jsp causes XSS; import org.owasp.encoder.Encode at the top of the JSP and replace direct scriptlet outputs for sdate and edate with encoded calls (use Encode.forHtml(sdate) and Encode.forHtml(edate)) where the template currently renders "<%=sdate%>" and "<%=edate%>" so all user-supplied date values are HTML-encoded before being written to the page.
235-241:⚠️ Potential issue | 🟡 MinorXSS vulnerability: Service codes and properties output without encoding.
All
vServiceCode,vServiceDesc, andprops.getProperty()values should be HTML-encoded before output.🛡️ Proposed fix
- <td align="center"><%=vServiceCode.get(i)%> + <td align="center"><%=Encode.forHtml((String)vServiceCode.get(i))%> </td> - <td><%=vServiceDesc.get(i)%> + <td><%=Encode.forHtml((String)vServiceDesc.get(i))%> </td> - <td align="center"><%=props.getProperty(vServiceCode.get(i) + "pat" + vServiceDesc.get(i))%> + <td align="center"><%=Encode.forHtml(props.getProperty(vServiceCode.get(i) + "pat" + vServiceDesc.get(i)))%> </td>Apply similar encoding to all remaining
props.getProperty()outputs in this table.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/report/reportbilledvisit2.jsp` around lines 235 - 241, Outputting vServiceCode, vServiceDesc and props.getProperty(...) directly creates XSS risk; HTML-encode these values before rendering. Replace the inline scriptlet expressions (vServiceCode.get(i), vServiceDesc.get(i), and props.getProperty(...)) with their HTML-escaped equivalents (e.g., use StringEscapeUtils.escapeHtml4(...) from Apache Commons Text or JSTL/EL <c:out> or fn:escapeXml) and apply the same encoding to all other props.getProperty() outputs in this table so every user-controlled value is escaped.src/main/java/ca/openosp/openo/form/FrmBCHPRecord.java (1)
80-86:⚠️ Potential issue | 🟠 MajorClose the new demographic
ResultSetin the edit branch.Line 81 opens a second
ResultSet, but theexistingID > 0path returns without closing it. That will leak JDBC resources on repeated form edits.Minimal fix
- ResultSet rs = DBHandler.GetPreSQL(sql, demographicNo); - if (rs.next()) { - props.setProperty("pg1_patientName_cur", rs.getString("last_name") + ", " + rs.getString("first_name")); - props.setProperty("pg1_phn_cur", rs.getString("hin")); - props.setProperty("pg1_phone_cur", rs.getString("phone") + " " + rs.getString("phone2")); - } + ResultSet rs = DBHandler.GetPreSQL(sql, demographicNo); + try { + if (rs.next()) { + props.setProperty("pg1_patientName_cur", rs.getString("last_name") + ", " + rs.getString("first_name")); + props.setProperty("pg1_phn_cur", rs.getString("hin")); + props.setProperty("pg1_phone_cur", rs.getString("phone") + " " + rs.getString("phone2")); + } + } finally { + rs.close(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/form/FrmBCHPRecord.java` around lines 80 - 86, The demographic ResultSet opened via DBHandler.GetPreSQL in FrmBCHPRecord (the variable rs used with props.setProperty("pg1_patientName_cur", ...), "pg1_phn_cur", "pg1_phone_cur") is not closed on the edit path that returns early; update the code to close rs before any return by either using a try-with-resources or ensuring rs.close() is called in a finally block (or immediately before the existingID > 0 return) so the ResultSet is always closed and JDBC resources are not leaked.src/main/java/ca/openosp/openo/form/FrmDischargeSummaryRecord.java (1)
231-243:⚠️ Potential issue | 🟠 MajorUse the selected
clientNamealias when populating the property.The query at Line 231 aliases the name as
clientName, but Line 243 still readspName. On the new-record path this will either leaveclientNameempty or fail with a column lookup error.Suggested fix
- props.setProperty("clientName", Misc.getString(rs, "pName")); + props.setProperty("clientName", Misc.getString(rs, "clientName"));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/form/FrmDischargeSummaryRecord.java` around lines 231 - 243, The code in FrmDischargeSummaryRecord uses an SQL alias "clientName" in the SELECT but then reads the wrong column "pName" from the ResultSet; update the ResultSet lookup where props.setProperty("clientName", ...) is set (in the block that uses ResultSet rs from DBHandler.GetPreSQL) to use Misc.getString(rs, "clientName") (or otherwise match the SELECT alias), ensuring the property key "clientName" is populated from the aliased column.src/main/java/ca/openosp/openo/form/FrmRourke2006Record.java (1)
67-87:⚠️ Potential issue | 🔴 CriticalClose the second demographic
ResultSet.The
ResultSetopened on Line 68 is never closed before the method returns. That leaks JDBC resources every time an existing Rourke record is loaded.🛠️ Proposed fix
if (rs.next()) { String rourkeVal = props.getProperty("c_pName", ""); String demoVal = Misc.getString(rs, "pName"); @@ if (!rourkeVal.equals(demoVal)) { props.setProperty("c_birthDate", demoVal); updated = "true"; } } + rs.close(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/form/FrmRourke2006Record.java` around lines 67 - 87, The ResultSet 'rs' returned by DBHandler.GetPreSQL in FrmRourke2006Record is not closed, leaking JDBC resources; update the code around the DBHandler.GetPreSQL(...) call and the block that reads from 'rs' to ensure 'rs' is closed in a finally block or try-with-resources (use try (ResultSet rs = DBHandler.GetPreSQL(sql, demographicNo)) { ... }) so that 'rs' is always closed after use, and remove any early returns that would skip closing.src/main/java/ca/openosp/openo/billings/ca/bc/MSP/MSPReconcile.java (1)
1204-1208:⚠️ Potential issue | 🟠 MajorClose the nested
ResultSetinside the rejected-bill loop.Line 1204 opens a new
ResultSetfor every rejected bill and never closes it. On larger remittance runs that leaks cursors/statements and can exhaust the connection before the outer report finishes.♻️ Suggested fix
- ResultSet rsDemo = DBHandler.GetPreSQL("select phone,phone2 from demographic where demographic_no = ?", b.demoNo); - if (rsDemo.next()) { - b.demoPhone = rsDemo.getString("phone"); - b.demoPhone2 = rsDemo.getString("phone2"); - } + try (ResultSet rsDemo = DBHandler.GetPreSQL( + "select phone,phone2 from demographic where demographic_no = ?", + b.demoNo)) { + if (rsDemo.next()) { + b.demoPhone = rsDemo.getString("phone"); + b.demoPhone2 = rsDemo.getString("phone2"); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/billings/ca/bc/MSP/MSPReconcile.java` around lines 1204 - 1208, The nested ResultSet rsDemo obtained via DBHandler.GetPreSQL inside the rejected-bill loop is never closed; wrap the call to DBHandler.GetPreSQL and the subsequent rsDemo.next()/getString(...) usage in a try-with-resources (or ensure rsDemo.close() in a finally) so the ResultSet is always closed after use; locate the code that declares ResultSet rsDemo and the assignments to b.demoPhone and b.demoPhone2 and ensure the resource is closed immediately after those reads to prevent cursor/statement leaks.
🟡 Minor comments (9)
src/main/java/ca/openosp/openo/commn/dao/Billing3rdPartyAddressDaoImpl.java-69-72 (1)
69-72:⚠️ Potential issue | 🟡 MinorNormalize
keywordin the non-search_namebranch.Line 72 turns
nullinto"null%", so an empty search on any allowlisted column silently returns no rows instead of following the empty-prefix behavior already used in thesearch_namepath.💡 Suggested change
} else { // Validate search_mode as a column name String safeSearchColumn = validColumns.contains(search_mode) ? search_mode : "company_name"; where = safeSearchColumn.concat(" like :searchMode"); - params.put("searchMode", keyword + "%"); + params.put("searchMode", (keyword == null ? "" : keyword) + "%"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/commn/dao/Billing3rdPartyAddressDaoImpl.java` around lines 69 - 72, The non-search_name branch in Billing3rdPartyAddressDaoImpl currently does params.put("searchMode", keyword + "%") which turns null into "null%"; normalize keyword the same way as in the search_name path by replacing null with empty string (or defaulting to "") before building the like pattern, then set params.put("searchMode", normalizedKeyword + "%"); keep the existing safeSearchColumn, where assignment, and use normalizedKeyword wherever keyword is used for the search pattern.src/main/webapp/decision/antenatal/antenatalplanner.jsp-101-101 (1)
101-101:⚠️ Potential issue | 🟡 MinorHandle invalid
formIdparse failures gracefully.Line 101 can throw
NumberFormatExceptionfor malformed input and produce a hard page failure. Consider validating first and returning a controlled error path.Suggested fix
- rsdemo = DBHandler.GetPreSQL("select * from formONAR where ID = ?", Integer.parseInt(form_no)); + Integer formId; + try { + formId = Integer.valueOf(form_no); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Invalid formId"); + } + rsdemo = DBHandler.GetPreSQL("select * from formONAR where ID = ?", formId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/decision/antenatal/antenatalplanner.jsp` at line 101, The code calls Integer.parseInt(form_no) directly when building the query with DBHandler.GetPreSQL, which can throw NumberFormatException for malformed or null input; wrap the parse in a validation and safe-parse block (check form_no for null/empty, use a try/catch around Integer.parseInt or use a helper parseIntSafe) and on failure take a controlled error path (e.g., set an error message/redirect or skip the DB call) before calling DBHandler.GetPreSQL; update the code paths that reference form_no/formId so they only call DBHandler.GetPreSQL when the parsed integer is valid.src/main/webapp/decision/antenatal/antenatalplannerprint.jsp-70-70 (1)
70-70:⚠️ Potential issue | 🟡 MinorAdd parse guard for
formIdbefore binding.Line 70 can throw
NumberFormatExceptionon invalid input. Use a guarded parse and fail cleanly.Suggested fix
- rsdemo = DBHandler.GetPreSQL("select * from formAR where ID = ?", Integer.parseInt(form_no)); + Integer formId; + try { + formId = Integer.valueOf(form_no); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Invalid formId"); + } + rsdemo = DBHandler.GetPreSQL("select * from formAR where ID = ?", formId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/decision/antenatal/antenatalplannerprint.jsp` at line 70, The call to Integer.parseInt(form_no) when creating rsdemo via DBHandler.GetPreSQL can throw NumberFormatException for null/invalid form_no; before binding, validate and safely parse form_no (e.g., check null/empty, use a try/catch around Integer.parseInt or Integer.valueOf) and handle failure by logging the invalid input and returning/forwarding an error response instead of calling DBHandler.GetPreSQL; update the code path that sets rsdemo to only call DBHandler.GetPreSQL with a guaranteed valid integer ID.src/main/java/ca/openosp/openo/billings/ca/bc/pageUtil/ViewWCB2Action.java-111-113 (1)
111-113:⚠️ Potential issue | 🟡 MinorGuard empty provider query results before indexing.
Line 113 assumes at least one row exists. If no provider matches, this throws and fails the action.
Suggested fix
- if (lstResults != null) { + if (lstResults != null && !lstResults.isEmpty()) { String[] providerData = (String[]) lstResults.get(0); this.setW_pracno(providerData[0]); this.setW_payeeno(providerData[1]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/billings/ca/bc/pageUtil/ViewWCB2Action.java` around lines 111 - 113, The code in ViewWCB2Action where SqlUtils.getQueryResultsList is called must guard against empty results before casting/indexing: in the method containing the lines with providerNo, check that lstResults is not null and not empty (e.g., lstResults.size() > 0) before doing String[] providerData = (String[]) lstResults.get(0); and handle the empty case (return an error/early return, set an appropriate error message, or use a default) so the action does not throw when no provider rows exist.src/main/java/ca/openosp/openo/report/pageUtil/GeneratePatientLetters2Action.java-91-102 (1)
91-102:⚠️ Potential issue | 🟡 MinorHandle missing
demosbefore iterating.Line 135 still assumes
demosis non-null. A request withoutdemoswill throw a runtime error.Suggested fix
- if (demos != null) { - for (String demo : demos) { - if (demo != null) { - SqlUtils.validateNumericId(demo, "demographic_no"); - } - } - } + if (demos == null || demos.length == 0) { + throw new IllegalArgumentException("Missing required parameter: demos"); + } + for (String demo : demos) { + if (demo != null) { + SqlUtils.validateNumericId(demo, "demographic_no"); + } + }Also applies to: 135-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/report/pageUtil/GeneratePatientLetters2Action.java` around lines 91 - 102, The code is iterating over demos without guarding against null in GeneratePatientLetters2Action; add a null check before any loop that accesses the demos array (the for (String demo : demos) block that calls SqlUtils.validateNumericId) so you only iterate when demos != null (or treat an absent demos as an empty array) and apply the same null-check pattern wherever demos is referenced to avoid runtime NPEs; also keep the existing id validation via SqlUtils.validateNumericId(id, "reportLetter") unchanged.src/main/java/ca/openosp/openo/billings/MSP/ExtractBean.java-159-161 (1)
159-161:⚠️ Potential issue | 🟡 MinorResultSet
rsis never closed — potential resource leak.The
ResultSetreturned byDBHandler.GetPreSQL()at line 159 is iterated but never explicitly closed. While the connection may eventually be cleaned up, failing to close the ResultSet can exhaust database cursors under load.Consider closing
rsafter the while loop completes (around line 299), or wrapping it in a try-with-resources if the API supports it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/billings/MSP/ExtractBean.java` around lines 159 - 161, The ResultSet returned by DBHandler.GetPreSQL(query, providerNo) (variable rs in ExtractBean) is never closed, risking cursor/resource leaks; modify the code to ensure rs is closed after use—either wrap the call and result-processing loop in a try-with-resources (try (ResultSet rs = DBHandler.GetPreSQL(...)) { ... }) or explicitly call rs.close() in a finally block after the while (rs.next()) loop in the same method, ensuring any SQLException is handled/logged and not swallowed.src/main/java/ca/openosp/openo/encounter/data/EctPatientData.java-53-54 (1)
53-54:⚠️ Potential issue | 🟡 MinorWrap
Integer.parseInt()in exception handling for unvalidated numeric conversion.Both
getProviderNo()(lines 53–54) andgetPatient()(lines 80–81) convert theString demographicNoparameter directly to an integer without catchingNumberFormatException. If a non-numeric value reaches these methods, an uncaught exception will propagate instead of returning an empty result like the existing SQLException handling does. Validate input upstream or catchNumberFormatExceptionlocally and return an empty result.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/encounter/data/EctPatientData.java` around lines 53 - 54, Both getProviderNo and getPatient call Integer.parseInt(demographicNo) without handling NumberFormatException; wrap the parseInt call in a try/catch to catch NumberFormatException for the demographicNo parameter and return the same empty/neutral result (and/or null) that the existing SQLException handling returns, optionally logging the invalid input; ensure you modify the parse in getProviderNo and getPatient so invalid non-numeric demographicNo does not propagate an exception but follows the existing empty-result flow.src/main/java/ca/openosp/openo/hospitalReportManager/dao/HRMDocumentDao.java-53-59 (1)
53-59:⚠️ Potential issue | 🟡 MinorReject unknown sort directions instead of defaulting to ASC.
Line 57 currently treats every non-
DESCvalue as ascending, so invalid input still changes query behavior instead of being ignored as the method contract says.Suggested fix
private static String getSafeOrderByFragment(String orderColumn, String orderDirection) { - if (StringUtils.isEmpty(orderColumn) || StringUtils.isEmpty(orderDirection)) { + if (StringUtils.isBlank(orderColumn) || StringUtils.isBlank(orderDirection)) { return ""; } - Map<String, String> fragments = "DESC".equalsIgnoreCase(orderDirection) ? ORDER_DESC_FRAGMENTS : ORDER_ASC_FRAGMENTS; + Map<String, String> fragments; + if ("ASC".equalsIgnoreCase(orderDirection)) { + fragments = ORDER_ASC_FRAGMENTS; + } else if ("DESC".equalsIgnoreCase(orderDirection)) { + fragments = ORDER_DESC_FRAGMENTS; + } else { + return ""; + } String fragment = fragments.get(orderColumn); return fragment != null ? fragment : ""; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/hospitalReportManager/dao/HRMDocumentDao.java` around lines 53 - 59, getSafeOrderByFragment currently treats any non-"DESC" orderDirection as ASC; change it to only accept "ASC" or "DESC" and return "" for any other values. In practice, update the logic in getSafeOrderByFragment so you first normalize orderDirection and explicitly check equalsIgnoreCase("DESC") or equalsIgnoreCase("ASC") to select ORDER_DESC_FRAGMENTS or ORDER_ASC_FRAGMENTS, and if neither matches return "" immediately; then look up the orderColumn in the chosen fragments and return fragment or "" as before.src/main/java/ca/openosp/openo/messenger/pageUtil/MsgViewMessageByPosition2Action.java-225-226 (1)
225-226:⚠️ Potential issue | 🟡 Minor
linkedsorting is currently a no-op.Line 226 orders by
m.messageid is null, butm.messageidcannot be null in this inner join (m.messageid = mapp.messageID). Requests withorderBy=linkedwill therefore just fall back tom.messageid desc. Map this key to the real linked-status expression, or drop it from the whitelist.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/ca/openosp/openo/messenger/pageUtil/MsgViewMessageByPosition2Action.java` around lines 225 - 226, The "linked" orderBy is a no-op because it uses "m.messageid is null" but m.messageid is never null due to the inner join; update the mapping in MsgViewMessageByPosition2Action where key == "linked" to use the actual linked-status expression (e.g., reference the mapping table column such as "mapp.messageID IS NOT NULL" for linked messages or "mapp.messageID IS NULL" for unlinked), or remove "linked" from the whitelist entirely; if you need to detect absence of a mapping change the join to a LEFT JOIN and then use "mapp.messageID IS NULL/IS NOT NULL" as appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5f638b26-c619-4338-934a-c3a6afab1123
📒 Files selected for processing (148)
database/mysql/importCasemgmt.javasrc/main/java/ca/openosp/openo/PMmodule/dao/ProviderDaoImpl.javasrc/main/java/ca/openosp/openo/billings/MSP/ExtractBean.javasrc/main/java/ca/openosp/openo/billings/ca/bc/MSP/CDMReminderHlp.javasrc/main/java/ca/openosp/openo/billings/ca/bc/MSP/CreateBillingReport2Action.javasrc/main/java/ca/openosp/openo/billings/ca/bc/MSP/MSPReconcile.javasrc/main/java/ca/openosp/openo/billings/ca/bc/pageUtil/ViewWCB2Action.javasrc/main/java/ca/openosp/openo/billings/ca/on/data/JdbcBillingReviewImpl.javasrc/main/java/ca/openosp/openo/commn/dao/Billing3rdPartyAddressDaoImpl.javasrc/main/java/ca/openosp/openo/commn/dao/BillingBCDaoImpl.javasrc/main/java/ca/openosp/openo/commn/dao/BillingONCHeader1Dao.javasrc/main/java/ca/openosp/openo/commn/dao/BillingONCHeader1DaoImpl.javasrc/main/java/ca/openosp/openo/commn/dao/DemographicDaoImpl.javasrc/main/java/ca/openosp/openo/commn/dao/DxDaoImpl.javasrc/main/java/ca/openosp/openo/commn/dao/DxresearchDAOImpl.javasrc/main/java/ca/openosp/openo/commn/dao/EFormReportToolDaoImpl.javasrc/main/java/ca/openosp/openo/commn/dao/InboxResultsRepositoryImpl.javasrc/main/java/ca/openosp/openo/commn/dao/PregnancyFormsDao.javasrc/main/java/ca/openosp/openo/commn/dao/SecurityDao.javasrc/main/java/ca/openosp/openo/commn/dao/SecurityDaoImpl.javasrc/main/java/ca/openosp/openo/commn/dao/SiteDaoImpl.javasrc/main/java/ca/openosp/openo/daos/LookupDaoImpl.javasrc/main/java/ca/openosp/openo/db/DBHandler.javasrc/main/java/ca/openosp/openo/eform/APExecute.javasrc/main/java/ca/openosp/openo/eform/EFormUtil.javasrc/main/java/ca/openosp/openo/eform/actions/FetchUpdatedData2Action.javasrc/main/java/ca/openosp/openo/eform/data/EForm.javasrc/main/java/ca/openosp/openo/encounter/data/EctARRecord.javasrc/main/java/ca/openosp/openo/encounter/data/EctAlphaRecord.javasrc/main/java/ca/openosp/openo/encounter/data/EctAnnualRecord.javasrc/main/java/ca/openosp/openo/encounter/data/EctMMSERecord.javasrc/main/java/ca/openosp/openo/encounter/data/EctMentalHealthRecord.javasrc/main/java/ca/openosp/openo/encounter/data/EctPalliativeCareRecord.javasrc/main/java/ca/openosp/openo/encounter/data/EctPatientData.javasrc/main/java/ca/openosp/openo/encounter/data/EctPeriMenopausalRecord.javasrc/main/java/ca/openosp/openo/encounter/data/EctRourkeRecord.javasrc/main/java/ca/openosp/openo/encounter/data/EctType2DiabetesRecord.javasrc/main/java/ca/openosp/openo/form/Frm2MinWalkRecord.javasrc/main/java/ca/openosp/openo/form/FrmARBloodWorkTest.javasrc/main/java/ca/openosp/openo/form/FrmAREnhancedBloodWorkTest.javasrc/main/java/ca/openosp/openo/form/FrmARRecord.javasrc/main/java/ca/openosp/openo/form/FrmAdfRecord.javasrc/main/java/ca/openosp/openo/form/FrmAdfV2Record.javasrc/main/java/ca/openosp/openo/form/FrmAnnualRecord.javasrc/main/java/ca/openosp/openo/form/FrmAnnualV2Record.javasrc/main/java/ca/openosp/openo/form/FrmBCAR2007Record.javasrc/main/java/ca/openosp/openo/form/FrmBCAR2012Record.javasrc/main/java/ca/openosp/openo/form/FrmBCARRecord.javasrc/main/java/ca/openosp/openo/form/FrmBCBirthSumMo2008Record.javasrc/main/java/ca/openosp/openo/form/FrmBCBrithSumMoRecord.javasrc/main/java/ca/openosp/openo/form/FrmBCClientChartChecklistRecord.javasrc/main/java/ca/openosp/openo/form/FrmBCHPRecord.javasrc/main/java/ca/openosp/openo/form/FrmBCINRRecord.javasrc/main/java/ca/openosp/openo/form/FrmBCNewBorn2008Record.javasrc/main/java/ca/openosp/openo/form/FrmBCNewBornRecord.javasrc/main/java/ca/openosp/openo/form/FrmCESDRecord.javasrc/main/java/ca/openosp/openo/form/FrmCaregiverRecord.javasrc/main/java/ca/openosp/openo/form/FrmConsultantRecord.javasrc/main/java/ca/openosp/openo/form/FrmCostQuestionnaireRecord.javasrc/main/java/ca/openosp/openo/form/FrmCounselingRecord.javasrc/main/java/ca/openosp/openo/form/FrmCounsellorAssessmentRecord.javasrc/main/java/ca/openosp/openo/form/FrmDischargeSummaryRecord.javasrc/main/java/ca/openosp/openo/form/FrmFallsRecord.javasrc/main/java/ca/openosp/openo/form/FrmGripStrengthRecord.javasrc/main/java/ca/openosp/openo/form/FrmGrowth0_36Record.javasrc/main/java/ca/openosp/openo/form/FrmGrowthChartRecord.javasrc/main/java/ca/openosp/openo/form/FrmHomeFallsRecord.javasrc/main/java/ca/openosp/openo/form/FrmImmunAllergyRecord.javasrc/main/java/ca/openosp/openo/form/FrmIntakeHxRecord.javasrc/main/java/ca/openosp/openo/form/FrmIntakeInfoRecord.javasrc/main/java/ca/openosp/openo/form/FrmInternetAccessRecord.javasrc/main/java/ca/openosp/openo/form/FrmInvoiceRecord.javasrc/main/java/ca/openosp/openo/form/FrmLabReq07Record.javasrc/main/java/ca/openosp/openo/form/FrmLabReq10Record.javasrc/main/java/ca/openosp/openo/form/FrmLabReqRecord.javasrc/main/java/ca/openosp/openo/form/FrmLateLifeFDIDisabilityRecord.javasrc/main/java/ca/openosp/openo/form/FrmLateLifeFDIFunctionRecord.javasrc/main/java/ca/openosp/openo/form/FrmMMSERecord.javasrc/main/java/ca/openosp/openo/form/FrmMentalHealthForm14Record.javasrc/main/java/ca/openosp/openo/form/FrmMentalHealthForm1Record.javasrc/main/java/ca/openosp/openo/form/FrmMentalHealthForm42Record.javasrc/main/java/ca/openosp/openo/form/FrmMentalHealthRecord.javasrc/main/java/ca/openosp/openo/form/FrmONAREnhancedRecord.javasrc/main/java/ca/openosp/openo/form/FrmONARRecord.javasrc/main/java/ca/openosp/openo/form/FrmOvulationRecord.javasrc/main/java/ca/openosp/openo/form/FrmPalliativeCareRecord.javasrc/main/java/ca/openosp/openo/form/FrmPeriMenopausalRecord.javasrc/main/java/ca/openosp/openo/form/FrmPolicyRecord.javasrc/main/java/ca/openosp/openo/form/FrmPositionHazardRecord.javasrc/main/java/ca/openosp/openo/form/FrmReceptionAssessmentRecord.javasrc/main/java/ca/openosp/openo/form/FrmRecordHelp.javasrc/main/java/ca/openosp/openo/form/FrmRhImmuneGlobulinRecord.javasrc/main/java/ca/openosp/openo/form/FrmRourke2006Record.javasrc/main/java/ca/openosp/openo/form/FrmRourke2009Record.javasrc/main/java/ca/openosp/openo/form/FrmRourkeRecord.javasrc/main/java/ca/openosp/openo/form/FrmSF36CaregiverRecord.javasrc/main/java/ca/openosp/openo/form/FrmSF36Record.javasrc/main/java/ca/openosp/openo/form/FrmSatisfactionScaleRecord.javasrc/main/java/ca/openosp/openo/form/FrmSelfAdministeredRecord.javasrc/main/java/ca/openosp/openo/form/FrmSelfAssessmentRecord.javasrc/main/java/ca/openosp/openo/form/FrmSelfEfficacyRecord.javasrc/main/java/ca/openosp/openo/form/FrmSelfManagementRecord.javasrc/main/java/ca/openosp/openo/form/FrmTreatmentPrefRecord.javasrc/main/java/ca/openosp/openo/form/FrmType2DiabeteRecord.javasrc/main/java/ca/openosp/openo/form/FrmchfRecord.javasrc/main/java/ca/openosp/openo/form/data/FrmData.javasrc/main/java/ca/openosp/openo/form/pageUtil/FormForward2Action.javasrc/main/java/ca/openosp/openo/form/pageUtil/FrmForm2Action.javasrc/main/java/ca/openosp/openo/form/pageUtil/FrmSetupForm2Action.javasrc/main/java/ca/openosp/openo/form/pageUtil/FrmXmlUpload2Action.javasrc/main/java/ca/openosp/openo/hospitalReportManager/dao/HRMDocumentDao.javasrc/main/java/ca/openosp/openo/hospitalReportManager/v2018/HRM2Action.javasrc/main/java/ca/openosp/openo/lab/ca/bc/PathNet/PathNetInfo.javasrc/main/java/ca/openosp/openo/managers/SecurityManager.javasrc/main/java/ca/openosp/openo/messenger/docxfer/send/MsgGenerate.javasrc/main/java/ca/openosp/openo/messenger/pageUtil/MsgViewMessageByPosition2Action.javasrc/main/java/ca/openosp/openo/report/ClinicalReports/SQLDenominator.javasrc/main/java/ca/openosp/openo/report/ClinicalReports/SQLNumerator.javasrc/main/java/ca/openosp/openo/report/data/RptReportConfigData.javasrc/main/java/ca/openosp/openo/report/data/RptReportCreator.javasrc/main/java/ca/openosp/openo/report/data/RptReportFilter.javasrc/main/java/ca/openosp/openo/report/data/RptReportItem.javasrc/main/java/ca/openosp/openo/report/pageUtil/GeneratePatientLetters2Action.javasrc/main/java/ca/openosp/openo/report/pageUtil/RptDownloadCSVServlet.javasrc/main/java/ca/openosp/openo/report/pageUtil/RptFormQuery.javasrc/main/java/ca/openosp/openo/report/reportByTemplate/ReportObject.javasrc/main/java/ca/openosp/openo/report/reportByTemplate/ReportObjectGeneric.javasrc/main/java/ca/openosp/openo/report/reportByTemplate/SQLReporter.javasrc/main/java/ca/openosp/openo/util/JDBCUtil.javasrc/main/java/ca/openosp/openo/util/PreparedSQL.javasrc/main/java/ca/openosp/openo/util/SqlUtils.javasrc/main/java/ca/openosp/openo/www/lookup/LookupCodeEdit2Action.javasrc/main/java/ca/openosp/openo/www/lookup/LookupCodeList2Action.javasrc/main/java/ca/openosp/openo/www/lookup/LookupList2Action.javasrc/main/webapp/admin/logReport.jspsrc/main/webapp/admin/securitysearchresults.jspsrc/main/webapp/billing/CA/BC/onSearch3rdBillAddr.jspsrc/main/webapp/billing/CA/ON/billingONNewReport.jspsrc/main/webapp/decision/antenatal/antenatalplanner.jspsrc/main/webapp/decision/antenatal/antenatalplannerprint.jspsrc/main/webapp/eform/efmformapconfig_lookup.jspsrc/main/webapp/oscarMDS/PatientSearch.jspsrc/main/webapp/report/reportResult.jspsrc/main/webapp/report/reportbilledvisit1.jspsrc/main/webapp/report/reportbilledvisit2.jspsrc/main/webapp/report/reportbilledvisit3.jspsrc/main/webapp/report/reportonbilledphcp.jspsrc/main/webapp/report/reportonbilledphcpv2.jsp
💤 Files with no reviewable changes (1)
- src/main/java/ca/openosp/openo/commn/dao/BillingONCHeader1Dao.java
There was a problem hiding this comment.
Sorry @LiamStanziani, your pull request is larger than the review limit of 150000 diff characters
Collaborator
Author
|
I want to do more testing on this branch after any new AI reviews come in, setting to ready for review to see any final comments, not specifically fully ready yet |
Review Summary by QodoSecurity: Phase 2 SQL Injection Remediation — Parameterized Queries & Input Validation Across 148+ Files
WalkthroughsDescription **Phase 2 SQL Injection Remediation** — Comprehensive hardening across 148+ files to eliminate SQL
injection vulnerabilities by converting string-concatenated queries to parameterized statements and
adding input validation for dynamic SQL identifiers.
**Core Changes:**
• **Parameterized Query Conversion**: Replaced string concatenation with PreparedStatement using
? placeholders across form records (~50 files), DAOs, report generators, billing modules, and JSPs
• **New Utility Infrastructure**:
- SqlUtils — validation methods (validateNumericId, validateTableName, validateColumnName,
validateSortColumn, inClausePlaceholders)
- PreparedSQL — value object pairing parameterized SQL with bind parameters
- DBHandler.GetPreSQL() / GetPreSQLUpdatable() — centralized parameterized query execution
- FrmRecordHelp — parameterized overloads for form record operations
• **Dynamic Identifier Whitelisting**: Added allowlist validation for table names, column names, and
ORDER BY clauses where parameterization is impossible
• **Report Template System**: New getParameterizedSQL() methods replace {param} markers with ?
placeholders, supporting single values, multi-value IN clauses, and empty checkbox cases
• **Billing & EForm Hardening**: Converted MSP/ON billing queries, EForm DatabaseAP templates, and
lookup actions to use parameterized queries with strict input validation
• **Deprecated Unsafe APIs**: Marked legacy methods (SecurityDao.findAllOrderBy,
FrmRecordHelp.getFormRecord(String)) as @Deprecated with security warnings
• **JPA Parameter Fixes**: Corrected positional parameter indexing (?1, ?2, etc.) in Hibernate
queries
**Validation Patterns Applied:**
• Numeric ID validation: ^[0-9]+$
• Table/column names: ^[A-Za-z0-9_]{1,10}$ with allowlist sets
• Sort columns: Hardcoded safe ORDER BY fragments via whitelisted maps
• Lookup table IDs: Strict regex ^[A-Za-z0-9]{1,10}$ before downstream parameterized queries
**Files Modified**: Form records (FrmAnnualRecord, FrmARRecord, FrmCaregiverRecord, etc.), DAOs
(DemographicDaoImpl, LookupDaoImpl, EFormReportToolDaoImpl, etc.), billing modules (MSPReconcile,
JdbcBillingReviewImpl, BillingONCHeader1DaoImpl), report system (ReportObjectGeneric, SQLReporter),
eForm system (EForm, APExecute, FetchUpdatedData2Action), and utility classes (SqlUtils, JDBCUtil,
EFormUtil)
**Security Impact**: Eliminates HIGH and MEDIUM severity SQL injection findings across multiple code
paths; all 20 remaining Snyk alerts are confirmed false positives due to unrecognized intermediate
validation and parameterization.
Diagramflowchart LR
A["String-Concatenated SQL<br/>User Input + Query"] -->|"Convert to<br/>Parameterized"| B["PreparedStatement<br/>with ? Placeholders"]
A -->|"Dynamic Identifiers<br/>Cannot Parameterize"| C["Whitelist Validation<br/>Table/Column Names"]
B --> D["DBHandler.GetPreSQL<br/>FrmRecordHelp<br/>SqlUtils"]
C --> D
D --> E["Safe Query Execution<br/>Bind Parameters Separated<br/>from SQL Logic"]
F["Report Templates<br/>EForm DatabaseAP<br/>Billing Queries"] -->|"New Utilities"| G["PreparedSQL<br/>getParameterizedSQL<br/>parameterizeFields"]
G --> E
H["Deprecated Unsafe APIs<br/>findAllOrderBy<br/>getFormRecord String"] -->|"Migration Path"| D
File Changes1. src/main/java/ca/openosp/openo/billings/ca/bc/MSP/MSPReconcile.java
|
Code Review by Qodo
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 148 files
Confidence score: 2/5
- High risk:
Set.of()insrc/main/java/ca/openosp/openo/eform/EFormUtil.javawill throw at runtime due to duplicate elements, which can crash form handling. - Security/behavior concerns:
src/main/java/ca/openosp/openo/daos/LookupDaoImpl.javaregex can allow unsafe SQL sequences, andsrc/main/java/ca/openosp/openo/billings/MSP/ExtractBean.javadrops the date-range filter causing unexpected data expansion. - Overall severity is elevated due to a definite runtime exception plus user-facing query/regression risks; proceed only after fixes.
- Pay close attention to
src/main/java/ca/openosp/openo/eform/EFormUtil.java,src/main/java/ca/openosp/openo/daos/LookupDaoImpl.java,src/main/java/ca/openosp/openo/billings/MSP/ExtractBean.java- crash risk, SQL safety, and missing filter behavior.
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/main/java/ca/openosp/openo/encounter/data/EctPatientData.java">
<violation number="1" location="src/main/java/ca/openosp/openo/encounter/data/EctPatientData.java:54">
P2: `Integer.parseInt(demographicNo)` is unhandled here; non-numeric input will throw `NumberFormatException` and bypass the existing `SQLException` handling.</violation>
</file>
<file name="src/main/java/ca/openosp/openo/form/FrmGrowth0_36Record.java">
<violation number="1" location="src/main/java/ca/openosp/openo/form/FrmGrowth0_36Record.java:27">
P2: Close this `ResultSet` in the `else` branch (prefer try-with-resources) to avoid leaking JDBC resources.</violation>
</file>
<file name="src/main/java/ca/openosp/openo/form/FrmGripStrengthRecord.java">
<violation number="1" location="src/main/java/ca/openosp/openo/form/FrmGripStrengthRecord.java:61">
P2: The `studyID` query is duplicated consecutively, causing an unnecessary extra DB call with no behavioral benefit.</violation>
</file>
<file name="src/main/java/ca/openosp/openo/commn/dao/DemographicDaoImpl.java">
<violation number="1" location="src/main/java/ca/openosp/openo/commn/dao/DemographicDaoImpl.java:2506">
P2: Guard against null `fieldName` before using `String.concat`; this new code will throw a NullPointerException if `fieldName` is null.</violation>
</file>
<file name="src/main/java/ca/openosp/openo/eform/EFormUtil.java">
<violation number="1" location="src/main/java/ca/openosp/openo/eform/EFormUtil.java:112">
P0: `Set.of()` throws `IllegalArgumentException` when given duplicate elements. Since `NAME` equals `"form_name"`, `SUBJECT` equals `"subject"`, etc., there are 5 duplicate pairs here. This will crash with `ExceptionInInitializerError` when the class is loaded, making all EForm functionality unusable.
Remove the duplicate string literals and keep only the constants plus the two extras (`"form_date"`, `"form_time"`) that don't have corresponding constants.</violation>
</file>
<file name="src/main/java/ca/openosp/openo/billings/MSP/ExtractBean.java">
<violation number="1" location="src/main/java/ca/openosp/openo/billings/MSP/ExtractBean.java:157">
P2: `dateRange` is still set via `setDateRange(...)`, but it is no longer applied to the billing query. This drops the date-range filter entirely and will return all open/waiting records for the provider instead of the requested range.</violation>
</file>
<file name="src/main/java/ca/openosp/openo/daos/LookupDaoImpl.java">
<violation number="1" location="src/main/java/ca/openosp/openo/daos/LookupDaoImpl.java:46">
P2: The `SAFE_SQL_EXPRESSION` regex allows spaces and parentheses together, which permits arbitrary SQL keyword sequences (e.g., `field) UNION SELECT secret FROM users WHERE (1`) to pass validation. For a pattern whose stated purpose is preventing second-order SQL injection from DB config, this is overly permissive. Consider either (a) disallowing spaces (using a strict identifier pattern with only `_`, `.`, `()`, `,`) or (b) adding an explicit keyword denylist.
Also, the Javadoc example `concat(first_name, ' ', last_name)` would fail this regex because `'` is not in the allowed character class — the comment should be corrected to match the actual behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Collaborator
Author
|
This should be good for review now. I have added a section for manually tested areas. I expect that overall testing with these changes would be good when testing out the general application, as many of these changes should be caught with that approach anyway. I expect this to be the same approach used for the Path Traversal and XSS fixes, since there will be so many changes that targeted testing of every change will most likely take too long, and should be delegated to when overall testing is done (so repeatedly testing areas of the project could cover many different changes at once for issues). |
Signed-off-by: Liam Stanziani <102267755+LiamStanziani@users.noreply.github.com>
D3V41
reviewed
Apr 10, 2026
D3V41
reviewed
Apr 10, 2026
D3V41
reviewed
Apr 10, 2026
D3V41
reviewed
Apr 10, 2026
D3V41
reviewed
Apr 10, 2026
D3V41
reviewed
Apr 10, 2026
D3V41
reviewed
Apr 10, 2026
D3V41
reviewed
Apr 10, 2026
D3V41
approved these changes
Apr 10, 2026
Collaborator
D3V41
left a comment
D3V41
left a comment
There was a problem hiding this comment.
Looks amazing, @LiamStanziani. Just one question: would it make sense to remove these deprecated methods and GetSQL() from the DBHandler? What do you think?
Collaborator
Author
|
Thank you! @D3V41, yeah I think that would be a good idea, I'll review soon, I assume they are reworked already so that these methods probably have no callers currently and will most likely be removed in a commit after i check over these. |
…s to work with sql parm version of the old deprecated methods, avoided removing DBHandler.GetSQL for now as the remaining calls are quite complex, and should probably be done in its own PR
Collaborator
Author
|
As an update @D3V41 I have removed the flagged methods you have mentioned in the review, and some other unused methods I found along the way, as well as updating the connections left connected to the old method to fit the PreSQL() method version. The only thing skipped was removing GetSQL() from DBHandler, the reason for this was that the remaining calls seemed to be quite complex dynamic report building, so I was thinking about fixing it in its own branch, let me know if you disagree with that. here is the commit if you would like to review the new changes: db43ea5 |
Collaborator
|
@LiamStanziani Reviewed it. We can handle the GetSQL() method another day. Overall, the PR looks good. Thank you. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 2 of SQL injection remediation across 148 files. Converts string-concatenated SQL to parameterized queries using PreparedStatement with ? bind parameters, and adds allowlist validation for dynamic SQL identifiers (table names, column names, sort orders) that cannot be parameterized.
Problem
Numerous DAO classes, form record classes, report generators, billing modules, and JSPs constructed SQL queries via string concatenation of user-supplied or request-derived values. This exposed the application to SQL injection attacks — flagged by Snyk as HIGH and MEDIUM severity findings across multiple code paths including:
Solution
Core utilities added/extended:
parameterization isn't possible (dynamic table/column names, IN clauses)
Parameterization approach by area:
? placeholders
single values, multi-value IN clauses, and empty checkbox cases
Set.of(...) before inclusion
Remaining Snyk findings: All 20 remaining SQL injection alerts are confirmed false positives — Snyk's taint analysis does not recognize intermediate
validation (regex allowlists, numeric validation) or parameterization (PreparedSQL, parameterizeFields) between the source and sink.
Areas tested manually
Path: Patient chart → eForms → Add eForm → select "Test DatabaseAP"
Verifies: EForm.parameterizeFields, EFormUtil.getValues, PreparedSQL, efmformapconfig_lookup.jsp
Expected: Patient name, DOB, age, address, provider auto-populate
Path: Main screen → Search bar → search by last name, try clicking column headers to sort
Verifies: DemographicDaoImpl.findByField, PatientSearch.jsp
Expected: Results appear, sorting works
Path: Patient chart → Forms → open any form (BC INR, Growth 0-36, Annual, Mental Health, BCAR, Rourke, Lab Req, etc.)
Verifies: FrmRecordHelp, FrmData.executeWithValidatedTable, all ~50 Frm*Record.java files
Expected: Form loads with patient data, can save without errors
Path: Report → Report by Template → pick a template → fill parameters → Generate
Verifies: ReportObjectGeneric.getParameterizedSQL, SQLReporter, DBHandler.GetPreSQL
Expected: Report generates results, no SQL errors
Path: Report → Clinical Reports → select denominator/numerator → Run
Verifies: SQLNumerator.executeParameterizedSQL, SQLDenominator.parameterizeAll
Expected: Report runs, patient list or counts display
Path: Administration → Log Report → select a provider → set date range → Submit. Also try "All Providers"
Verifies: logReport.jsp all three query branches
Expected: Log entries display
Path: Billing → ON → Billing Report
Verifies: billingONNewReport.jsp, reportbilledvisit1/2/3.jsp, reportonbilledphcp.jsp
Expected: Reports generate with correct data
Path: Msg → view messages → try sorting by different columns
Verifies: MsgViewMessageByPosition2Action
Expected: Messages display, sorting works
Path: Patient chart → Forms → AR form → Scroll to the bottom of the page → click "AR Planner" link
Verifies: antenatalplanner.jsp, antenatalplannerprint.jsp
Expected: Planner loads or gracefully handles missing data
Path: Administration → User Management → Search/Edit/Delete Security Records → Search for a providers number or username
Verifies: securitysearchresults.jsp, SecurityDaoImpl
Expected: Search results display
Path: Patient chart → E-Chart → click through left navbar panels (Annual, Mental Health, MMSE, Palliative Care, Peri-Menopausal, Rourke, Type 2
Diabetes, AR)
Verifies: All Ect*Record.java files, EctPatientData
Expected: Each panel loads patient data without errors
Path: Patient chart → E-Chart → Disease Registry → add or view a diagnosis
Verifies: DxDaoImpl, DxresearchDAOImpl
Expected: Diagnosis search and list works
Path: Main screen → Inbox → view lab results, documents
Verifies: InboxResultsRepositoryImpl
Expected: Inbox items load and display
Path: Billing → ON → review billing records
Verifies: JdbcBillingReviewImpl, BillingONCHeader1DaoImpl
Expected: Billing review displays records
Path: Report → Ontario Prevention Report → Query for demographic results → Scroll to the bottom of the page → Generate Patient Letters
Verifies: GeneratePatientLetters2Action
Expected: Letter generation works
Path: Patient chart → Forms → open a form via shortcut name, or navigate between forms
Verifies: FormForward2Action, FrmForm2Action, FrmSetupForm2Action
Expected: Forms open to the correct form type for the patient
Path: Patient chart → Forms → ONAR or pregnancy-related form
Verifies: PregnancyFormsDao
Expected: Form loads pregnancy data
Path: Administration → Reports → EForm Report Tool → Add New eForm
Verifies: EFormReportToolDaoImpl
Expected: Report queries work
Summary by cubic
Phase 2 SQL injection hardening across billing, eForms, DAOs, and form records. Replaced concatenated SQL with parameterized queries, validated dynamic identifiers, added safe ORDER BY/IN builders, and removed unused deprecated SQL helpers. DatabaseAP templates now use
PreparedSQL; most paths useDBHandler.GetPreSQL/GetPreSQLUpdatableand parameterizedFrmRecordHelp.ParameterizedCriteria); validatedrano; fixed provider/payee and service code lookups; updated WCB view/reprocess actions; removed unsafe invoice report methods inJdbcBillingReviewImpl.APExecute/EFormbuildPreparedSQLfrom templates (stripped template quotes);EFormUtiladds ORDER BY whitelist with tiebreakers;FetchUpdatedData2Actionvalidatesdemographic,provider,uuid.SecurityDao.findAllOrderBy(String)in favor offindAllOrderByUserName; deletedBillingONCHeader1Dao.findBillingData; added whitelists/validation and parameterized queries acrossDemographicDaoImpl,DxresearchDAOImpl,DxDaoImpl,ProviderDaoImpl,BillingBCDaoImpl,Billing3rdPartyAddressDaoImpl,InboxResultsRepositoryImpl,EFormReportToolDaoImpl,LookupDaoImpl,PregnancyFormsDao; parameterizedimportCasemgmt.Frm*/Ect*records toDBHandler.GetPreSQL;FrmRecordHelp.getFormRecord(sql, ...)binds params and maps types safely; addedDBHandler.GetPreSQLUpdatablefor updatable cursors.Written for commit db43ea5. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores