SQL injection in typeORM

[maxdiebold-erg]

A SQL injection vulnerability is affecting the following repositories:

• easey-common
• easey-monitor-plan-api
• easey-streaming-services
• easey-mdm-api
• easey-facilities-api
• easey-account-api
• easey-auth-api
• easey-emissions-api
• easey-qa-certification-api
• easey-camd-services

These dependabot alerts will update the typeorm package, but the affected @nestjs packages must be manually updated. The typeorm update has breaking changes, so changes will need to be made in the code, primarily find -> findBy, findOne -> findOneBy, & updating custom repositories to remove the EntityRepository decorator.

[maxdiebold-erg]

I have linked all of the PRs related to this ticket, but the only one currently ready for review is easey-common: once that PR is merged in, I can update the @us-epa-camd/easey-common package version in the other repositories, and they can be tested & merged.

[maxdiebold-erg]

The easey-common updates have been merged, and the package has been updated in all other repositories, so the other Pull Requests are now ready for review.

[esaber76]

1. Receive the error below attempting to import a QA test from historical.

2. Receive the error below "in public view" when expanding a facility to get to the locations on every screen (Monitoring Plans, Test Data, etc.). Seeing the same error when using admin tool. Also when attempting to download reports.

3. Receive the error below attempting to add a Protocol Gas Record for a Linearity.

4. Receive the error below attempting to add a Linearity Test Record for a Linearity.

5. Receive the error below attempting to import a QA test from file (Example file for ORIS 6137, unit 3: QA & Certification _ Export - A B Brown Generating Station (1).json).

6. Unable to save a Test Summary Record for a RATA, Flow to Load Check, Flow to Load Reference, Fuel Flow to Load Test, Fuel Flow to Load Baseline, Unit Default, Miscellaneous test in the QA screen. Clicking on Save and Close after entering data in does nothing. This behavior does not exist on tst.

7. Receive the error below attempting to add a Cycle Time Injection Record for a Cycle Time test.

8. When attempting to Edit a Test Summary record for a Cycle Time Test, the Test Type Code dropdown does not have any options.

9. Receive the error below attempting to add a Transmitter Transducer Accuracy Data Record.

10. Receive the error below attempting to add a Unit Default Protocol Gas Record.

11. Receive the error below attempting to add a Unit Default Air Emission Record.

12. For Monitoring Plans, incorrect Eval Status and Submission Status appearing on Evaluate and Submit screens. I made no changes to this monitoring plan example. It should be showing Passed/Updated on Host not Needs Evaluation/Submitted with Critical Errors.

[maxdiebold-erg]

It looks like 10 pull requests is the limit for linking to issues, so I'll add more here:

• https://github.com/US-EPA-CAMD/easey-qa-certification-api/pull/593
• https://github.com/US-EPA-CAMD/easey-monitor-plan-api/pull/544
• https://github.com/US-EPA-CAMD/easey-emissions-api/pull/458
• https://github.com/US-EPA-CAMD/easey-camd-services/pull/234
• https://github.com/US-EPA-CAMD/easey-camd-services/pull/235

[maxdiebold-erg]

@lgiannini1 @esaber76 I do not think problem number 8 noted above (When attempting to Edit a Test Summary record for a Cycle Time Test, the Test Type Code dropdown does not have any options) is TypeORM-related, it seems to be related to state management in the interface. Also, I was only able to reproduce it once. If either of you have a reliable way to reproduce the problem, it may be better suited for a different ticket.

[esaber76]

Think this may be related to TypeORM. Receiving this error when selecting the Download button for any reports.

[ibarra-michelle]

Issue: Connection default error when importing MATS file in ECMPS dev

@maxdiebold-erg, this morning, I received the Connection default error message when importing a MATS file for Comanche (470), Location 1:

09:14:30.929: [APP/PROC/WEB.0] [Nest] 142  - 05/23/2024, 9:14:30 AM   ERROR [/camd-services/mats-file-upload/MDC-75702FF835394F518216D79BEDE6EFA6/1/LINE/EPA-106-1002/import] Connection "default" was not found.
09:14:30.929: [APP/PROC/WEB.0] ConnectionNotFoundError: Connection "default" was not found.
09:14:30.929: [APP/PROC/WEB.0] at ConnectionManager.get (/home/vcap/deps/0/node_modules/typeorm/connection/ConnectionManager.js:43:19)
09:14:30.929: [APP/PROC/WEB.0] at getConnection (/home/vcap/deps/0/node_modules/typeorm/globals.js:90:35)
09:14:30.929: [APP/PROC/WEB.0] at MatsBulkFile.getRepository (/home/vcap/deps/0/node_modules/typeorm/repository/BaseEntity.js:70:79)
09:14:30.929: [APP/PROC/WEB.0] at MatsBulkFile.create (/home/vcap/deps/0/node_modules/typeorm/repository/BaseEntity.js:105:21)
09:14:30.929: [APP/PROC/WEB.0] at MatsFileUploadService.importFile (/home/vcap/app/dist/mats-file-upload/mats-file-upload.service.js:64:73)
09:14:30.929: [APP/PROC/WEB.0] at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
09:14:30.929: [APP/PROC/WEB.0] at async MatsFileUploadController.uploadFile (/home/vcap/app/dist/mats-file-upload/mats-file-upload.controller.js:41:9)
09:14:30.930: [APP/PROC/WEB.0] {
09:14:30.930: [APP/PROC/WEB.0] path: '/camd-services/mats-file-upload/MDC-75702FF835394F518216D79BEDE6EFA6/1/LINE/EPA-106-1002/import',
09:14:30.930: [APP/PROC/WEB.0] stack: 'ConnectionNotFoundError: Connection "default" was not found.\n' +
09:14:30.930: [APP/PROC/WEB.0] '    at ConnectionManager.get (/home/vcap/deps/0/node_modules/typeorm/connection/ConnectionManager.js:43:19)\n' +
09:14:30.930: [APP/PROC/WEB.0] '    at getConnection (/home/vcap/deps/0/node_modules/typeorm/globals.js:90:35)\n' +
09:14:30.930: [APP/PROC/WEB.0] '    at MatsBulkFile.getRepository (/home/vcap/deps/0/node_modules/typeorm/repository/BaseEntity.js:70:79)\n' +
09:14:30.930: [APP/PROC/WEB.0] '    at MatsBulkFile.create (/home/vcap/deps/0/node_modules/typeorm/repository/BaseEntity.js:105:21)\n' +
09:14:30.930: [APP/PROC/WEB.0] '    at MatsFileUploadService.importFile (/home/vcap/app/dist/mats-file-upload/mats-file-upload.service.js:64:73)\n' +
09:14:30.930: [APP/PROC/WEB.0] '    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)\n' +
09:14:30.930: [APP/PROC/WEB.0] '    at async MatsFileUploadController.uploadFile (/home/vcap/app/dist/mats-file-upload/mats-file-upload.controller.js:41:9)',
09:14:30.930: [APP/PROC/WEB.0] statusCode: 500,
09:14:30.930: [APP/PROC/WEB.0] errorId: '138c8af2-e935-4b1f-8ee8-c7c3d17697a3',
09:14:30.930: [APP/PROC/WEB.0] level: 'error',
09:14:30.930: [APP/PROC/WEB.0] message: 'Connection "default" was not found.'
09:14:30.930: [APP/PROC/WEB.0] }

[maxdiebold-erg]

@esaber76 @ibarra-michelle Thank you for catching those, they should now be fixed in dev.

[ibarra-michelle]

@esaber76 @ibarra-michelle Thank you for catching those, they should now be fixed in dev.

@maxdiebold-erg I was able to submit MATS files and receive the success message. I no longer see the connection default error message, thank you.

[lgiannini1]

All errors noted above in the QA screens have been resolved.

[maxdiebold-erg]

Pull requests related to explicit null values in find* queries:

• https://github.com/US-EPA-CAMD/easey-common/pull/144
• https://github.com/US-EPA-CAMD/easey-auth-api/pull/199
• https://github.com/US-EPA-CAMD/easey-monitor-plan-api/pull/547
• https://github.com/US-EPA-CAMD/easey-qa-certification-api/pull/595
• https://github.com/US-EPA-CAMD/easey-camd-services/pull/236
• https://github.com/US-EPA-CAMD/easey-emissions-api/pull/460

[djw4erg]

Monitor Plan Management Configuration Error

Receiving the following with endpoint call https://api.epa.gov/easey/dev/monitor-plan-mgmt/configurations/last-updated?date=2023-08-01, which previously worked.

{
    "message": "Connection \"default\" was not found.",
    "errorId": "daccee46-e70d-49fc-8347-bda158d9102b",
    "statusCode": 500
}

The following link indicates a TypeORM issue. https://stackoverflow.com/questions/49794140/connection-default-was-not-found-with-typeorm

[lgiannini1]

Verified that QA test exports no longer have a componentId entered for system-level tests or a monitoringSystemId for component-level tests.

[maxdiebold-erg]

In the previous changes, I overlooked one breaking change noted in the release notes:

findOne and QueryBuilder.getOne() now return null instead of undefined in the case if it didn't find anything in the database. Logically it makes more sense to return null.

This means that all explicit comparisons of a result against undefined should be changed to null (alternatively, since the method will return either a complex type or null, one can just check if it is falsy). The relevant pull requests are linked below:

• https://github.com/US-EPA-CAMD/easey-qa-certification-api/pull/597
• https://github.com/US-EPA-CAMD/easey-facilities-api/pull/181
• https://github.com/US-EPA-CAMD/easey-camd-services/pull/238
• https://github.com/US-EPA-CAMD/easey-emissions-api/pull/461
• https://github.com/US-EPA-CAMD/easey-monitor-plan-api/pull/549

[esaber76]

@maxdiebold-erg - I was working with Max K. on something and he got this error when trying to add a QA Cert Event. Is this a typeORM issue.

[maxdiebold-erg]

@esaber76 Not on the surface, but I'd want to check it out. I didn't get that error when adding a QA cert event to ORIS 63259 DEPC2, so it'd be nice to get the details to reproduce the error.

[esaber76]

@maxdiebold-erg Don't worry about it. I just tried it again for other facilities and the same facility and it was fine. If it comes up again, we can circle back.

[ntknguyen]

@maxdiebold-erg - Michelle and I were looking at ticket CAMPD ticket for additional information on how we could test this ECMPS ticket. We just need your confirmation that the following steps are needed for the testing.

1. Check to see if the typeorm version in package.json has been updated to 0.3.0

2. findBy and findOneBy

For unit test files, xyz, do these need to be change find -> findBy, findOne -> findOneBy ? Thuy and I found text using the deprecated function.

3. IsNull() How should we test this? Should we focus on the files which imported IsNull from the typeORM package?

search terms: from 'typeorm' or from "typeorm"

4. Removal of the EntityRepository decorator

Similar to the CAMPD ticket #6290, should we search for the @EntityRepository decorartor does not exist and ensure the @Injectable decorator does exist, where applicable?

5. getManager() Should we review whether getManager() was removed from the repos? The CAMPD ticket indicates getManager() function should be removed.

Thanks

[maxdiebold-erg]

@ntknguyen I think at this point regression-testing as many features of the application is the best way to test the underlying code changes. I put more information into the CAMPD ticket to provide direction based on experience with this ticket.

If you'd like to review the code, then you are correct that the TypeORM version in package.json should be 0.3.0, the @EntityRepository decorator should be completely removed, and the getManager function should be completely removed. The signatures of the find and findOne functions were changed, but they were not deprecated, so they may still be found throughout the code. IsNull should be used whenever a null value is intentionally passed to the query (testing the presence of IsNull won't help much, unfortunately, because errors would more likely stem from queries where it is missed).

[ntknguyen]

@ntknguyen I think at this point regression-testing as many features of the application is the best way to test the underlying code changes. I put more information into the CAMPD ticket to provide direction based on experience with this ticket.

If you'd like to review the code, then you are correct that the TypeORM version in package.json should be 0.3.0, the @EntityRepository decorator should be completely removed, and the getManager function should be completely removed. The signatures of the find and findOne functions were changed, but they were not deprecated, so they may still be found throughout the code. IsNull should be used whenever a null value is intentionally passed to the query (testing the presence of IsNull won't help much, unfortunately, because errors would more likely stem from queries where it is missed).

Thanks for your notes @maxdiebold-erg. I verify that this issue is fixed.