-
Notifications
You must be signed in to change notification settings - Fork 23
[G2PCCODV-50] Kafka Streams #77
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.
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
base: master
Are you sure you want to change the base?
Conversation
Added inflight parsers
* include Gov-360 update * include Gov-325 update * update tenants config * remove extra fields from transfer * add bpmn config bulk_processor_account_lookup * update variable name * import phee-494 changes from master * fix gov-325 additions * fix deduplication issues * update pipeline * temp remove checkstyle command * remove schema_name
* Add variable * Added log to check content of recordDoecument * Added logs and updated repository to findByBatchId and subBatchId * coorected syntax * Updated code to handle dublication issue in batches * updated batchId setup logic * updated saving logic * updated saving logic * Used optional --------- Co-authored-by: Apurb Rajdhan <[email protected]> Co-authored-by: abhinav <[email protected]>
* Adding bpmn * add variable * tenant * add correlation id variable * Adding addtional variables
* variable fix * Adding logs * Adding states * change * changes * refactor code * test * log * log * log * log * log * fix * fix * add * cleanup * change * condition
* Debugging reverted hikari cond * Debugging reverted hikari cond * Debugging reverted hikari cond * Debugging reverted hikari cond * Debugging reverted hikari cond * Resolved Hikaripool issue * Testing transaction handling change * Testing transaction handling change * Testing transaction handling change --------- Co-authored-by: Abhinav Mishra <[email protected]>
* add variable * log * tets * tets * clean
* Add condition * comment condition * adding enum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Jit has detected 1 important finding in this PR that you should review.
The finding is detailed below as a comment.
It’s highly recommended that you fix this security issue before merge.
|
||
if (Strings.isNotBlank(transformer.getXpath())) { | ||
logger.debug("applying xpath for variable {}", variableName); | ||
Document document = documentBuilderFactory.newDocumentBuilder().parse(new InputSource(new StringReader(variableValue))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security control: Static Code Analysis Java
Type: Java.Lang.Security.Audit.Xxe.Documentbuilderfactory-Disallow-Doctype-Decl-Missing.Documentbuilderfactory-Disallow-Doctype-Decl-Missing
Description: DOCTYPE declarations are enabled for this DocumentBuilderFactory. This is vulnerable to XML external entity attacks. Disable this by setting the feature "http://apache.org/xml/features/disallow-doctype-decl" to true. Alternatively, allow DOCTYPE declarations and only prohibit external entities declarations. This can be done by setting the features "http://xml.org/sax/features/external-general-entities" and "http://xml.org/sax/features/external-parameter-entities" to false.
Severity: HIGH
Jit Bot commands and options (e.g., ignore issue)
You can trigger Jit actions by commenting on this PR review:
#jit_ignore_fp
Ignore and mark this specific single instance of finding as “False Positive”#jit_ignore_accept
Ignore and mark this specific single instance of finding as “Accept Risk”#jit_undo_ignore
Undo ignore command
* Populating payee dfsp id for payerFundTransfer flow * Addressing review comments * Populating payee fsp id in pay bb * Update variable name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Jit has detected 5 important findings in this PR that you should review.
The findings are detailed as separate comments.
It’s highly recommended that you fix these security issues before merge.
api 'com.fasterxml.jackson.dataformat:jackson-dataformat-csv:2.14.0' | ||
api 'com.jayway.jsonpath:json-path:2.7.0' | ||
api 'mysql:mysql-connector-java:8.0.31' | ||
api 'org.postgresql:postgresql:42.5.1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security control: Software Component Analysis Gradle
Type: Org.Postgresql:Postgresql Vulnerable To Sql Injection Via Line Comment Generation (Bit-Postgresql-Jdbc-Driver-2024-1597, Cve-2024-1597, Cve-2024-32888, GHSA-x3wm-hffr-chwm)
Description: # Impact
SQL injection is possible when using the non-default connection property preferQueryMode=simple
in combination with application code that has a vulnerable SQL that negates a parameter value.
There is no vulnerability in the driver when using the default query mode. Users that do not override the query mode are not impacted.
Exploitation
To exploit this behavior the following conditions must be met:
- A placeholder for a numeric value must be immediately preceded by a minus (i.e.
-
) - There must be a second placeholder for a string value after the first placeholder on the same line.
- Both parameters must be user controlled.
The prior behavior of the driver when operating in simple query mode would inline the negative value of the first parameter and cause the resulting line to be treated as a --
SQL comment. That would extend to the beginning of the next parameter and cause the quoting of that parameter to be consumed by the comment line. If that string parameter includes a newline, the resulting text would appear unescaped in the resulting SQL.
When operating in the default extended query mode this would not be an issue as the parameter values are sent separately to the server. Only in simple query mode the parameter values are inlined into the executed SQL causing this issue.
Example
PreparedStatement stmt = conn.prepareStatement("SELECT -?, ?");
stmt.setInt(1, -1);
stmt.setString(2, "\nWHERE false --");
ResultSet rs = stmt.executeQuery();
The resulting SQL when operating in simple query mode would be:
SELECT --1,'
WHERE false --'
The contents of the second parameter get injected into the command. Note how both the number of result columns and the WHERE clause of the command have changed. A more elaborate example could execute arbitrary other SQL commands.
Patch
Problem will be patched upgrade to 42.7.2, 42.6.1, 42.5.5, 42.4.4, 42.3.9, 42.2.28, 42.2.28.jre7
The patch fixes the inlining of parameters by forcing them all to be serialized as wrapped literals. The SQL in the prior example would be transformed into:
SELECT -('-1'::int4), ('
WHERE false --')
Workarounds
Do not use the connection propertypreferQueryMode=simple
. (NOTE: If you do not explicitly specify a query mode then you are using the default of extended
and are not impacted by this issue.)
Severity: CRITICAL
Jit Bot commands and options (e.g., ignore issue)
You can trigger Jit actions by commenting on this PR review:
#jit_ignore_fp
Ignore and mark this specific single instance of finding as “False Positive”#jit_ignore_accept
Ignore and mark this specific single instance of finding as “Accept Risk”#jit_undo_ignore
Undo ignore command
api 'org.yaml:snakeyaml:2.0' | ||
api 'org.springframework.boot:spring-boot-actuator:3.0.9' | ||
api 'org.apache.kafka:kafka-streams:3.3.2' | ||
api 'org.springframework.kafka:spring-kafka:3.0.9' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security control: Software Component Analysis Gradle
Type: Spring-Kafka Has Java Deserialization Vulnerability When Improperly Configured (Cve-2023-34040)
Description: In Spring for Apache Kafka 3.0.9 and earlier and versions 2.9.10 and earlier, a possible deserialization attack vector existed, but only if unusual configuration was applied. An attacker would have to construct a malicious serialized object in one of the deserialization exception record headers.
Specifically, an application is vulnerable when all of the following are true:
- The user does not configure an ErrorHandlingDeserializer for the key and/or value of the record
- The user explicitly sets container properties checkDeserExWhenKeyNull and/or checkDeserExWhenValueNull container properties to true.
- The user allows untrusted sources to publish to a Kafka topic
By default, these properties are false, and the container only attempts to deserialize the headers if an ErrorHandlingDeserializer is configured. The ErrorHandlingDeserializer prevents the vulnerability by removing any such malicious headers before processing the record.
Severity: HIGH
Jit Bot commands and options (e.g., ignore issue)
You can trigger Jit actions by commenting on this PR review:
#jit_ignore_fp
Ignore and mark this specific single instance of finding as “False Positive”#jit_ignore_accept
Ignore and mark this specific single instance of finding as “Accept Risk”#jit_undo_ignore
Undo ignore command
api 'org.springframework.boot:spring-boot-actuator:3.0.9' | ||
api 'org.apache.kafka:kafka-streams:3.3.2' | ||
api 'org.springframework.kafka:spring-kafka:3.0.9' | ||
api 'org.json:json:20230227' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security control: Software Component Analysis Gradle
Type: Java: Dos Vulnerability In Json-Java (Cve-2023-5072)
Description: ### Summary
A denial of service vulnerability in JSON-Java was discovered by ClusterFuzz. A bug in the parser means that an input string of modest size can lead to indefinite amounts of memory being used. There are two issues: (1) the parser bug can be used to circumvent a check that is supposed to prevent the key in a JSON object from itself being another JSON object; (2) if a key does end up being a JSON object then it gets converted into a string, using \
to escape special characters, including \
itself. So by nesting JSON objects, with a key that is a JSON object that has a key that is a JSON object, and so on, we can get an exponential number of \
characters in the escaped string.
Severity
High - Because this is an already-fixed DoS vulnerability, the only remaining impact possible is for existing binaries that have not been updated yet.
Proof of Concept
package orgjsonbug;
import org.json.JSONObject;
/**
* Illustrates a bug in JSON-Java.
*/
public class Bug {
private static String makeNested(int depth) {
if (depth == 0) {
return "{\"a\":1}";
}
return "{\"a\":1;\t\0" + makeNested(depth - 1) + ":1}";
}
public static void main(String[] args) {
String input = makeNested(30);
System.out.printf("Input string has length %d: %s\n", input.length(), input);
JSONObject output = new JSONObject(input);
System.out.printf("Output JSONObject has length %d: %s\n", output.toString().length(), output);
}
}
When run, this reports that the input string has length 367. Then, after a long pause, the program crashes inside new JSONObject with OutOfMemoryError.
Further Analysis
The issue is fixed by this PR.
Timeline
Date reported: 07/14/2023
Date fixed:
Date disclosed: 10/12/2023
Severity: HIGH
Jit Bot commands and options (e.g., ignore issue)
You can trigger Jit actions by commenting on this PR review:
#jit_ignore_fp
Ignore and mark this specific single instance of finding as “False Positive”#jit_ignore_accept
Ignore and mark this specific single instance of finding as “Accept Risk”#jit_undo_ignore
Undo ignore command
@@ -1,6 +1,5 @@ | |||
FROM openjdk:13 | |||
EXPOSE 8000 | |||
FROM openjdk:17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security control: Docker Scan
Type: Image User Should Not Be 'Root'
Description: Running containers with 'root' user can lead to a container escape situation. It is a best practice to run containers as non-root users, which can be done by adding a 'USER' statement to the Dockerfile.
Severity: HIGH
Fix suggestion:
This fix suggestion was generated by Jit. Please note that the suggestion might not always fit every use case. It is highly recommended that you check and review it before merging.
Suggestion guidelines
- First of all, check if your container is running as a root user. In most of the cases, you can do it by running a command like this:
docker run <image> whoami
. If it returnsroot
, then you should consider using a non-root user, by following one of the next steps:- If a non-root user already exists in your container, consider using it.
- If not, you can create a new user by adding a
USER
command to the Dockerfile, with a non-root user as argument, for example:USER <non-root-user-name>
.
FROM openjdk:17 | |
FROM openjdk:17 | |
RUN addgroup --system <group> | |
RUN adduser --system <user> --ingroup <group> | |
USER <user>:<group> | |
Jit Bot commands and options (e.g., ignore issue)
You can trigger Jit actions by commenting on this PR review:
#jit_ignore_fp
Ignore and mark this specific single instance of finding as “False Positive”#jit_ignore_accept
Ignore and mark this specific single instance of finding as “Accept Risk”#jit_undo_ignore
Undo ignore command
Description
(Ignore if these details are present on the associated JIRA ticket)
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Design related bullet points or design document link related to this PR added in the description above.
Updated corresponding Postman Collection or Api documentation for the changes in this PR.
Create/update unit or integration tests for verifying the changes made.
Add required Swagger annotation and update API documentation with details of any API changes if applicable
Followed the naming conventions as given in https://docs.google.com/document/d/1Q4vaMSzrTxxh9TS0RILuNkSkYCxotuYk1Xe0CMIkkCU/edit?usp=sharing