checkmarx-ltd/checkmarx-spring-boot-java-sdk

checkmarx-spring-boot-java-sdk's Issues

SAST results should include scan-level custom fields

SAST 9.4.0 introduced scan-level custom fields. The CxService class' getReportContent method should retrieve the scan-level custom fields (if they are present) and add them to the additionalDetails member of the ScanResults class. As the project-level custom fields are already stored in the customFields property, maybe the scan-level custom fields could be stored in a scanCustomFields property.

Race condition when creating a CxSAST project

There is a race condition in the CxService.createScan(CxScanParams params, String comment) method between the testing for a project's existence and CxFlow's subsequent creation of the project in the case that it does not already exist. It is possible that two, or more, CxFlow instances can check for the project's existence more or less simultaneously and all believe that the project doesn't exist and so try to create it. Only one instance will be able to successfully create the project; all others will fail.

The CxSAST REST API returns a specific error code for this case:

{
    "messageCode": 12108,
    "messageDetails": "Project name already exists"
}

The createProject(String ownerId, String name) method should be enhanced to detect the above error and, if encountered, retrieve the existing project's ID.

The CxService.getProjectId method cannot handle project names with an embedded #

The CxService class' getProjectId method invokes the exchange method of the Spring framework's RestTemplate class in the following way:

            ResponseEntity<String> projects = restTemplate.exchange(cxProperties.getUrl().concat(PROJECTS)
                    .concat("?projectName=").concat(name).concat("&teamId=").concat(ownerId), HttpMethod.GET, httpEntity, String.class);

If the project name includes an embedded '#', the Spring framework, internally, views this as the start of a fragment (see https://en.wikipedia.org/wiki/URL) or, more specifically still, the fromUriString method of the UriComponentsBuilder class: https://github.com/spring-projects/spring-framework/blob/v5.3.6/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java).

A possible solution is to use the UriComponents and UriComponentsBuilder classes directly. Something like:

            UriComponents uriComponents = UriComponentsBuilder
                .fromHttp(cxProperties.getUrl())
                .path({PROJECTS)
                .queryParam("projectName", name)
                .queryParam("teamId", ownerId)
                .build();
            ResponseEntity<String> projects = restTemplate.exchange(uriComponents.toUri(), HttpMethod.GET, httpEntity, String.class);

I believe the same problem will affect all methods where we build up a URL that includes query parameters, including:

• getLastScanId
• getLastScanDate
• getProjectId
• getScanIdOfExistingScanIfExists

The CxService.getTeamId method should ignore case when looking for a team

The CxService.getTeamId method uses the equals method to compare teams:

            for (CxTeam team : teams) {
                if (team.getFullName().equals(teamPath)) {
                    log.info(FOUND_TEAM, teamPath, team.getId());
                    return team.getId();
                }
            }

However, in Checkmarx, team names are treated as being case-insensitive (e.g. it will not allow the creation of two teams whose names differ only by case). As such, this method should use the equalsIgnoreCase method.

CxService.setProjectRepositoryDetails has hard-coded error message

The setProjectRepositoryDetails method (in the CxService class) has a hard-coded error message for when the request fails:

        try {
            log.info("Updating Source details for project Id {}", projectId);
            restTemplate.exchange(cxProperties.getUrl().concat(PROJECT_SOURCE), HttpMethod.POST, requestEntity, String.class, projectId);
        } catch (HttpStatusCodeException e) {
            log.error("Error occurred while updating Project source info for project {}.", projectId);
            throw new CheckmarxException("Error occurred while adding source details to project.  Please ensure GIT is defined within Checkmarx");
        }

This should be changed to report the error message from the response body. For example, given the following response:

{
    "messageCode": 12331,
    "messageDetails": "Failure occurred during GIT Browsing"
}

CxFlow should report the messageDetails (and, possibly, the messageCode) in the log.

CxSAST queuing feature doesn't have any logs

There isn't any logging when waiting for scan completion when queuing scans in the SDK backend, currently, logging in queuing is limited to when the timeout is reached.

We need to have informative logging at least in debug mode so we can understand if we're waiting for a scan to finish before committing a new one or else we can't even understand if we even reach the queuing phase.

My proposal:

1. Add INFO logging once a scan is queued in the logs here with the scan ID outputted:

   • checkmarx-spring-boot-java-sdk/src/main/java/com/checkmarx/sdk/service/CxService.java

     Line 2387 in 19384c3
2. Add debug logging for each 5 or 10 minutes informing that the scan with the given ID of the current project hasn't finished yet here:

   • checkmarx-spring-boot-java-sdk/src/main/java/com/checkmarx/sdk/service/CxService.java

     Line 2393 in 19384c3

This is also related to CxFlow issue 905 enhancement since PRs are not receiving prompt Markdown feedback on whether a scan was queued and/or is still in queue, so that CxFlow knows that the scan is still waiting in queue to be triggered in CxSAST.

Team path separator Issue with CxGo

If the team ends with a \ when using cxgo, the processing will fail:
org.springframework.web.client.HttpClientErrorException$BadRequest: 400 Bad Request: [The business application with the same name already exists]

This works

  team: \businessUnit

This does not

  team: \businessUnit\

OutOfMemoryError exception thrown from trace log output when trying to base 64 encode a 250MB XML report

The exception below appears when running CxFlow 1.6.9 that includes the 0.4.43 SDK. The XML report in this case is ~250MB.

2021-04-20 15:16:32.786 ERROR 26578 --- [           main] c.c.f.CxFlowRunner                        [46jiUSeb] : An error occurred while processing request

java.util.concurrent.CompletionException: java.lang.OutOfMemoryError: Requested array size exceeds VM limit
	at org.springframework.aop.interceptor.AsyncExecutionAspectSupport.lambda$doSubmit$3(AsyncExecutionAspectSupport.java:279)
	at java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1604)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.OutOfMemoryError: Requested array size exceeds VM limit
	at java.lang.StringCoding$StringEncoder.encode(StringCoding.java:300)
	at java.lang.StringCoding.encode(StringCoding.java:344)
	at java.lang.StringCoding.encode(StringCoding.java:387)
	at java.lang.String.getBytes(String.java:958)
	at com.checkmarx.sdk.service.CxService.getReportContent(CxService.java:392)
	at com.checkmarx.sdk.service.CxService.getReportContentByScanId(CxService.java:365)
	at com.checkmarx.flow.service.ResultsService.processScanResultsAsync(ResultsService.java:60)
	at com.checkmarx.flow.service.ResultsService$$FastClassBySpringCGLIB$$2328645f.invoke(<generated>)
	at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:218)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:771)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:749)
	at org.springframework.aop.framework.adapter.MethodBeforeAdviceInterceptor.invoke(MethodBeforeAdviceInterceptor.java:56)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:175)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:749)
	at org.springframework.aop.aspectj.AspectJAfterAdvice.invoke(AspectJAfterAdvice.java:47)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:175)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:749)
	at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:95)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:749)
	at org.springframework.aop.interceptor.AsyncExecutionInterceptor.lambda$invoke$0(AsyncExecutionInterceptor.java:115)
	at org.springframework.aop.interceptor.AsyncExecutionInterceptor$$Lambda$508/727497138.call(Unknown Source)
	at org.springframework.aop.interceptor.AsyncExecutionAspectSupport.lambda$doSubmit$3(AsyncExecutionAspectSupport.java:276)
	at org.springframework.aop.interceptor.AsyncExecutionAspectSupport$$Lambda$509/889422145.get(Unknown Source)
	... 4 common frames omitted

Looking at the code, which still exists (at different line numbers), it is a line where a trace log line is being output with the base64 encoding of the XML report:

log.trace("Base64: {}", Base64.getEncoder().encodeToString(resultsXML.toString().getBytes()));

This appears in src/main/java/com/checkmarx/sdk/service/CxService.java.

At this location in the code, there are several debug/trace outputs there that are potentially high memory/cpu utilization that should be guarded by isDebugEnabled to avoid execution when not in debug mode. It would be better to not attempt to base64 encode the XML report for output given the XML report could be an arbitrary size. Even if isDebugEnabled is checked before output, this limits the ability to turn on debug logging given a large report could start causing failures in debug mode.

evelop

Large XML Reports cause crash due to OOM

Background

When fetching a report that is large (e.g. 500MB+) CxFlow can throw out of memory exceptions and crash. This is more common in memory constrained environments such as Github Actions where 7GB of memory is available for the task, even when that available memory is allocated for JVM heap space via JVM flags.

Suggested Solution
The reports need to be processed in a more memory efficient manner to prevent these crashes so that large reports will not crash CxFlow.

Details

Based on several stack traces, the issue occurs within getReportContent here: https://github.com/checkmarx-ltd/checkmarx-spring-boot-java-sdk/blob/develop/src/main/java/com/checkmarx/sdk/service/CxService.java#L381

The XML report is treated as a String type and several operations are performed on the string creating several pieces of garbage for the GC. When the report is large, these pieces of garbage are equally large and will quickly consume all available heap space.

The XML report is read into memory as a string:
https://github.com/checkmarx-ltd/checkmarx-spring-boot-java-sdk/blob/develop/src/main/java/com/checkmarx/sdk/service/CxService.java#L395

This line creates two pieces of garbage by trimming the string twice. This commonly triggers the OOM error.

https://github.com/checkmarx-ltd/checkmarx-spring-boot-java-sdk/blob/develop/src/main/java/com/checkmarx/sdk/service/CxService.java#L400

In other log examples, this function also causes OOM:
https://github.com/checkmarx-ltd/checkmarx-spring-boot-java-sdk/blob/develop/src/main/java/com/checkmarx/sdk/service/CxService.java#L402

Scan launched before project branching completes

In the case that the SDK creates a branched project, it is possible that it will launch a scan before the branching process completes. In recent versions of SAST, this will lead to the scan being canceled:

2022-11-29 09:41:47,555 [47] INFO - ScanRequest with id 1685335 was canceled. Branching process is not yet completed.

In older versions of SAST, I am told the scan would simply fail.

The SDK should be enhanced to address this issue. Newer versions of SAST provide an API endpoint for checking the branching status (GET /projects/branch/{id}) - if available, this endpoint should be used to prevent the scan being launched before branching has completed. For versions of SAST that do not have this endpoint, a configurable pause could be added after the branch request has been made.

SAST results should include the result detection date

Recent versions of SAST (9.3+) include the detection date of each result. This field is present in the XML report consumed by the SDK. For example (scroll to far right):

<Result NodeId="10001970002" FileName="test/main.py" Status="New" Line="7" Column="18" FalsePositive="False" Severity="High" AssignToUser="" state="2" Remark="CX Admin jsb-mig-test-tal-demo-project, [Monday, March 28, 2022 6:26:36 AM]: Changed status to Confirmed" DeepLink="http://ec2amaz-3tsivek/CxWebClient/ViewerMain.aspx?scanid=1000197&amp;projectid=149&amp;pathid=2" SeverityIndex="3" StatusIndex="2" DetectionDate="5/31/2022 12:11:53 AM">

The SDK should be enhanced to support this field.

Any changes should be backwards compatible (i.e., should work with SAST 9.2 and earlier).

Add a default constructor to XIssue

Currently, Jackson is unable to deserialize a JSON to ScanResults.class unless you use the ParameterNamesModule.

This is because XIssue lacks an empty constructor. Please add an empty constructor XIssue(). Please do the same for OsaDetails and ScaDetails

The affected file is https://github.com/checkmarx-ltd/checkmarx-spring-boot-java-sdk/blob/develop/src/main/java/com/checkmarx/sdk/dto/ScanResults.java