CAMPD v1.5 scan result: emissions-api

[alangmaid]

CAMPD_Emissions_API_Vulns.xlsx

[maheese]

@spetros-do I was looking at some of the 500 errors from the scans. There were quite a few instances. Here's a link to a Kibana search that filters the 500 errors in staging on the day of the scans. https://logs.fr.cloud.gov/goto/8fc1339faf959d3e9bcc5b476e94d534

A couple observations:

1. The validation for the query parameters, which are enumerations is not handled properly. It appears that whatever is passed by the user in the query string is being used in a database query to check if in fact that value is one of the emumerated values. When the scanner injects certain characters into these parameters, the query fails with the following stack trace:

QueryFailedError: invalid byte sequence for encoding "UTF8": 0x00 at PostgresQueryRunner.query (/home/vcap/deps/0/node_modules/typeorm/driver/postgres/PostgresQueryRunner.js:211:19) at process.processTicksAndRejections (node:internal/process/task_queues:95:5) at async SelectQueryBuilder.loadRawResults (/home/vcap/deps/0/node_modules/typeorm/query-builder/SelectQueryBuilder.js:2007:25) at async SelectQueryBuilder.executeEntitiesAndRawResults (/home/vcap/deps/0/node_modules/typeorm/query-builder/SelectQueryBuilder.js:1867:26) at async SelectQueryBuilder.getRawAndEntities (/home/vcap/deps/0/node_modules/typeorm/query-builder/SelectQueryBuilder.js:637:29) at async SelectQueryBuilder.getOne (/home/vcap/deps/0/node_modules/typeorm/query-builder/SelectQueryBuilder.js:664:25) at async DbLookupValidator.validate (/home/vcap/deps/0/node_modules/@us-epa-camd/easey-common/validators/db-lookup.validator.js:24:27) at async Promise.all (index 0) at async Promise.all (index 0) at async ValidationPipe.transform (/home/vcap/deps/0/node_modules/@nestjs/common/pipes/validation.pipe.js:64:24)

Since the issue is in the dblookup validator in commons this will effect every API where this is used.

2. When a non-numeric value is supplied for a year the application throws an exception that results in a 500 error. Example request:

/emissions-mgmt/emissions/apportioned/ozone/nationally?stateCode=AL&facilityId=100&unitType=Arch-fired+boiler&unitFuelType=Coal&controlTechnologies=Additives+to+Enhance+PAC+and+Existing+Equipment+Performance&programCodeInfo=ARP&year=100)+%22+WAITFOR+DELAY+%2700%3a00%3a10%27+--+&page=100&perPage=100

Resulting exception:

LoggingException: invalid input syntax for type numeric: "100) " WAITFOR DELAY '00:00:10' --" at OzoneApportionedEmissionsService.getEmissionsNationalAggregation (/home/vcap/app/dist/apportioned-emissions/ozone/ozone-apportioned-emissions.service.js:75:19) at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

[spetros-do]

Thank you, @maheese. That will definitely help in narrowing the issue(s) at hand.

[spetros-do]

@maheese, @mark-hayward-erg, @maxdiebold-erg, @szintgraff, @alangmaid

I. For the Db-lookup.validator ⇒ Issues Identified on the [src/validators/db-lookup.validator.ts]

• Lacks Input Sanitization - neither the original nor the updated code sanitizes the input before querying the database. This is critical to prevent SQL injection or passing invalid values (e.g., non-UTF-8 characters) to the query as observed on security scan.
• No Pre-Validation of Input - both codes do not validate the format of value (e.g., ensuring it is a number, string, or follows certain constraints) before performing the database lookup. This will lead to unnecessary database queries with invalid data, potentially leading to 500 errors. (#6405 comes to mind)
• Handling of SQL Injection - both queries use EntityManager.findOne(), which should be safe if properly parameterized, but there is no explicit protection against SQL injection in both the codes.
• Handling Empty Inputs - both codes return “true” if the ignoreEmpty option is set, even if no value is provided. This could be fine depending on the business logic intended, but in cases where one wants to enforce a value being required, this behavior could allow incorrect values to pass through.

⇒ Suggested changes: [Implement]

• Input Sanitization - to ensure that potentially dangerous characters are removed before querying the database.
• Pre-Validation – to check that the value follows the expected format (e.g., alphanumeric) before the query, reducing unnecessary database queries.
• Error Handling - such that it can throw clear, user-friendly errors when invalid input is detected.
• Safe Database Queries: Uses findOne with parameterized queries to protect against SQL injection.

II. For the Valid-Code Validator ⇒ Issues identified on the
[src/validators/is-valid-codes.validator.ts]

• No substantial change when comparing the original code vs the updated one - they both do the following:
• Checks if the value is a string.
• Splits the value by "," or "|"and removes any empty strings.
• No changes were made to the core validation logic or error handling mechanisms.

⇒ Suggested changes: [Implement - similar to above]

III. Next Steps ⇒

• Link/Reopen Tickets & PR’s and update suggested changes for both validators.
• Confirm with specific Test Cases - with valid values (existing in the database), invalid values (not existing in the database), empty strings, spaces, and null values, with different types of input (numeric, string, and for injection attempts…. (e.g.…. WAITFOR DELAY '00:00:10' --).

[spetros-do]

For #2: Operational Year Parameter (Op & Year) Issue: Based on the given example provided:

• this vulnerability was exposed when a security scan sent a malformed request with SQL injection payload in the year parameter, resulting in an application error.

The issue highlights the need for:

• robust input validation,
• proper type handling, and
• secure coding practices in processing user-supplied data

Specifically:

1. Lack of proper input validation -- The API accepts and attempts to process any input provided for the year parameter without adequate checks.
2. Incorrect type handling -- The system tries to treat the input as a numeric value without proper type conversion or validation.
3. SQL Injection vulnerability -- Due to the lack of input sanitization, it's possible to inject SQL commands through this parameter, potentially allowing unauthorized access to or manipulation of the database.
4. Error handling issues -- When invalid input is provided, the system returns detailed error messages that could expose sensitive information about the application's structure.
5. Inconsistent parameter handling - The system doesn't properly account for valid variations in input format, such as single years versus arrays of years.

Proposed Solution:

1. Enhanced Input Validation: [ ] Implement rigorous validation for the op&year parameter, ensuring it only accepts valid year formats (single year or range of years) within an appropriate historical range.
2. Type Safety: [ ] Introduce proper type checking and conversion to ensure the op&year input is always treated as the correct data type before being used in database queries.
3. Parameterized Queries: [ ] Adopt the use of parameterized queries or prepared statements for all database operations involving the op&year parameter, effectively preventing SQL injection attempts.
4. Middleware Implementation: [ ] Develop and deploy a request validation middleware that intercepts and sanitizes all incoming requests, applying consistent validation across the API.
5. Flexible Input Handling: [ ] Modify the system to correctly process both single year inputs and arrays of years, accommodating various valid input formats.
6. Improved Error Handling: [ ] Implement comprehensive error handling that catches and logs issues without exposing sensitive system information in error messages returned to users.