Skip to content

some-tests-for-review-tuning #35707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mbiuki wants to merge 1 commit into
base: main
Choose a base branch
from
+13 −32
Draft

some-tests-for-review-tuning #35707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 9 additions & 15 deletions dotCMS/src/main/java/com/dotcms/publisher/business/PublishAuditAPIImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,29 +225,23 @@ public PublishAuditStatus getPublishAuditStatus(String bundleId)
@CloseDBIfOpened
public List<PublishAuditStatus> getPublishAuditStatuses(List<String> bundleIds)
throws DotPublisherException {
if (bundleIds == null || bundleIds.isEmpty()) {
return Collections.emptyList();
}
try {
final List<PublishAuditStatus> result = new ArrayList<>();

final DotConnect dc = new DotConnect();
final String placeholders = bundleIds.stream()
.map(id -> "?")
.collect(Collectors.joining(","));
DotConnect dc = new DotConnect();
final List<String> parameter = bundleIds.stream().map(id -> "'" + id + "'").collect(Collectors.toList());

dc.setSQL(String.format(SELECT_ALL_BY_BUNDLES_IDS, placeholders));
bundleIds.forEach(dc::addParam);
final List<Map<String, Object>> items = dc.loadObjectResults();
dc.setSQL(String.format(SELECT_ALL_BY_BUNDLES_IDS, String.join(",", parameter)));
Copy link
Copy Markdown
Contributor

@semgrep-code-dotcms-test semgrep-code-dotcms-test Bot May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semgrep identified a blocking 🔴 issue in your code:

SQL query is built with String.format and unescaped bundleIds, allowing a crafted ID with a quote to break out of the string and return all rows (e.g., "x') OR 1=1 --").

More details about this

Risk: SQL built with String.format(...) uses untrusted bundleIds directly.

In getPublishAuditStatuses, bundleIds are wrapped in single quotes (parameter = bundleIds.stream().map(id -> "'" + id + "'")...) and then spliced into the query string via String.format(SELECT_ALL_BY_BUNDLES_IDS, String.join(",", parameter)). If any id contains a single quote, it breaks out of the string literal and alters the WHERE clause.

Concrete exploit

  • Step 1: An attacker passes a crafted ID to any API/controller that calls getPublishAuditStatuses, e.g. bundleIds = ["x') OR 1=1 --"].
  • Step 2: parameter becomes ["'x') OR 1=1 --'"] and the final SQL (assuming SELECT_ALL_BY_BUNDLES_IDS ≈ "... WHERE bundle_id IN (%s)") becomes:
    ... WHERE bundle_id IN ('x') OR 1=1 --')
  • Step 3: dc.setSQL(...) followed by dc.loadObjectResults() executes this query, making OR 1=1 true and returning all rows from the audit table (bypassing intended filters and exposing data).

Because the query is assembled from raw strings (SELECT_ALL_BY_BUNDLES_IDS + String.join on parameter) instead of using parameters, a malicious bundleId can inject SQL and control the query outcome.

To resolve this comment:

✨ Commit fix suggestion

Suggested change
dc.setSQL(String.format(SELECT_ALL_BY_BUNDLES_IDS, String.join(",", parameter)));
// Build a list of '?' placeholders based on the number of bundleIds
String placeholders = bundleIds.stream().map(id -> "?").collect(Collectors.joining(","));
String newQueryString = String.format(SELECT_ALL_BY_BUNDLES_IDS, placeholders);
dc.setSQL(newQueryString);
// Safely bind each bundleId to a parameter in the query
for (String id : bundleIds) {
dc.addParam(id);
}
View step-by-step instructions
  1. Refactor the code to use parameterized queries instead of building the SQL string with String.format and joining parameters.
  2. Update the SQL in SELECT_ALL_BY_BUNDLES_IDS to use placeholders (?) for each bundle ID, such as IN (?, ?, ?) instead of injecting the raw values.
  3. In your method, after building the query string with the correct number of placeholders based on bundleIds.size(), call dc.setSQL(newQueryString) without directly inserting the IDs.
  4. Loop through each value in bundleIds and call dc.addParam(id) for each one to bind them safely.

For example, if bundleIds.size() is 3, your SQL should look like ... IN (?, ?, ?) and you should call dc.addParam(bundleIds.get(0)); dc.addParam(bundleIds.get(1)); ....

This prevents SQL injection by letting the database driver escape all parameters safely.

💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by CUSTOM_INJECTION-2.

If this is a critical or high severity finding, please also link this issue in the #security channel in Slack.

You can view more details about this finding in the Semgrep AppSec Platform.

List<Map<String, Object>> items = dc.loadObjectResults();

for (final Map<String, Object> item : items) {
result.add(turnIntoPublishAuditStatus(NO_LIMIT_ASSETS, item));
for(Map<String, Object> item: items) {
result.add(turnIntoPublishAuditStatus(NO_LIMIT_ASSETS, item));
}

return result;
} catch (Exception e) {
Logger.error(PublishAuditAPIImpl.class, e.getMessage(), e);
throw new DotPublisherException("Unable to get list of elements with error:" + e.getMessage(), e);
}catch(Exception e){
Logger.debug(PublisherUtil.class,e.getMessage(),e);
throw new DotPublisherException("Unable to get list of elements with error:"+e.getMessage(), e);
}

}
Expand Down
1 change: 0 additions & 1 deletion dotCMS/src/main/java/com/dotcms/publisher/business/PublisherQueueJob.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,6 @@ private List<PublishAuditHistory> getRemoteHistoryFromEndpoint(final List<Strin

final String responseBody = webTarget
.request(MediaType.APPLICATION_JSON)
.header("Authorization", AuthCredentialPushPublishUtil.INSTANCE.getRequestToken(targetEndpoint).orElse(""))
.post(Entity.entity(bundleIds, MediaType.APPLICATION_JSON))
.readEntity(String.class);

Expand Down
20 changes: 4 additions & 16 deletions dotCMS/src/main/java/com/dotcms/rest/AuditPublishingResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,18 @@
import com.dotcms.publisher.business.DotPublisherException;
import com.dotcms.publisher.business.PublishAuditAPI;
import com.dotcms.publisher.business.PublishAuditStatus;
import com.dotcms.publisher.pusher.AuthCredentialPushPublishUtil;

import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.*;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;

import com.dotmarketing.util.Config;
import com.dotmarketing.util.Logger;
import com.google.common.collect.Lists;
import io.swagger.v3.oas.annotations.parameters.RequestBody;
import io.swagger.v3.oas.annotations.tags.Tag;

import java.util.List;
import java.util.Optional;

@Path("/auditPublishing")
@Tag(name = "Publishing")
Expand Down Expand Up @@ -54,18 +53,7 @@ public Response get(@PathParam("bundleId") final String bundleId,
@POST
@Path("/getAll")
@Produces(MediaType.APPLICATION_JSON)
public Response getAll(final List<String> bundleIds,
@Context final HttpServletRequest request) {

final AuthCredentialPushPublishUtil.PushPublishAuthenticationToken ppAuthToken =
AuthCredentialPushPublishUtil.INSTANCE.processAuthHeader(request);

final Optional<Response> failResponse = PushPublishResourceUtil.getFailResponse(request, ppAuthToken);

if (failResponse.isPresent()) {
return failResponse.get();
}

public Response getAll( List<String> bundleIds) {
try {
final List<PublishAuditStatus> statuses = auditAPI.getPublishAuditStatuses(bundleIds);

Expand Down
Loading