Fix code scanning alerts

[jordanpadams]

Tracking issue for:

• https://github.com/NASA-PDS/registry-common/security/code-scanning/35
• https://github.com/NASA-PDS/registry-common/security/code-scanning/36
• https://github.com/NASA-PDS/registry-common/security/code-scanning/37
• https://github.com/NASA-PDS/registry-common/security/code-scanning/38

These issues are unlikely to occur, but we should sanitize these messages in the event there is a man-in-middle attack or something where harvest/registry-mgr queries some faked "OpenSearch" or something like that.

[alexdunnjpl]

In order

1. Log message is a JSON dump. Unless sanitizing out newlines is sufficient sanitization, the risk must be accepted or the log message removed
2. Same as previous
3. The input-derived variables can be newline-stripped, or the LIDVID can be filtered/replaced with a regex and the message can be stripped of problematic characters (which? we don't know prima-facie all characters which should be allowed, so we can't regex filter)
4. Same as previous

@jordanpadams could you please weigh in on these?

[jordanpadams]

In order

1. Log message is a JSON dump. Unless sanitizing out newlines is sufficient sanitization, the risk must be accepted or the log message removed
2. Same as previous
3. The input-derived variables can be newline-stripped, or the LIDVID can be filtered/replaced with a regex and the message can be stripped of problematic characters (which? we don't know prima-facie all characters which should be allowed, so we can't regex filter)
4. Same as previous

@alexdunnjpl per the comment in the resolution for the log injection, can we do something like this? If it doesn't make sense, that is fine too. Just want to make sure we are addressing it if we can.

        // The regex check here, allows only alphanumeric characters to pass.
        // Hence, does not result in log injection
        if (username.matches("\\w*")) {
        }

[alexdunnjpl]

@jordanpadams that allowlist approach is most-desirable, but relies on knowing all possible allowable content and being able to differentiate between allowable and invalid content prima facie (with failures being logged as generic messages without the user-supplied value)

In the case of arbitrary JSON content, that could be anything, and is very likely to include line breaks, so it's necessary to identify all undesirable characters and remove them - there's no point in retaining the log message otherwise because its sole purpose is to dump the content for viewing.

I tend to agree with your comments about these being trivial attacks predicated upon an already-existing Very Bad Situation. The exception being this log which is amenable to just stripping out offending characters or using an allowlist approach.

[alexdunnjpl]

@alexdunnjpl to close individual issues and note justification in each case

[tloubrieu-jpl]

@alexdunnjpl @jordanpadams , similar alerts still show in the analysis report, see https://github.com/NASA-PDS/registry-common/security/code-scanning . Does something need to be done ?

[tloubrieu-jpl]

Note that @gxtchen came up with that question above during I&T.

[alexdunnjpl]

I don't know why those are just coming up as of two days ago... will look into the individual messages. What's the level of urgency?

[jordanpadams]

Per the log injection stuff, I think we just need to review them and either remove the logging from the code, or close them out as no longer a risk.

[tloubrieu-jpl]

The issues where either new lines or code or because the code changed the line numbers were different and the alerts were raised again. I went though them oe by one and dismissed them all.