fix: Measurement Plug-In Client Generator Python code injection

[Jotheeswaran-Nandagopal]

• [x] This contribution adheres to CONTRIBUTING.md.

What does this Pull Request accomplish?

• Add repr() function to the string parameters in the mako template to omit arbitrary Python code in the output.

Why should this Pull Request be merged?

Bug 2843797

• If any malicious service returns metadata with escape sequences, that results in arbitrary Python code injection in the output. This implementation handles the escape sequences with repr().

What testing has been done?

• Tested manually.

[Jotheeswaran-Nandagopal]

When a malicious service returns metadata with escape sequences, that leads to vulnerability. E.g., "This is a malicious function.\"\n import os; os.system(\"rm -rf \")\n test=\""

In this implementation,

• The parameters in the mako templates, which returns string type are represented with repr() function, as the escape sequences in above example are considered as lin separation and ends up in arbitrary code.
• The parameters of 'json' type are handling escape sequences by default.

The below is the image of the escape sequence handled output of the above example:

@bkeryan , I would like to know your suggestion on this fix for this Python code injection Bug.

[github-actions[bot]]

Test Results

    30 files  ±0      30 suites  ±0   38m 48s :stopwatch: -39s    651 tests ±0     651 :white_check_mark: ±0      0 :zzz: ±0  0 :x: ±0  16 130 runs  ±0  15 060 :white_check_mark: ±0  1 070 :zzz: ±0  0 :x: ±0 

Results for commit 91a22b40. ± Comparison against base commit 00d81038.

:recycle: This comment has been updated with latest results.

[bkeryan]

@bkeryan , I would like to know your suggestion on this fix for this Python code injection Bug.

I think using repr() is a good solution when you want to print a string literal. When you want to print an identifier, I think it's better to use isidentifier() and throw an error if the string isn't an identifier. For strings in comments, I think it's ok to use repr() and strip the quotes.

[Jotheeswaran-Nandagopal]

I think using repr() is a good solution when you want to print a string literal. When you want to print an identifier, I think it's better to use isidentifier() and throw an error if the string isn't an identifier. For strings in comments, I think it's ok to use repr() and strip the quotes.

Sure @bkeryan. I have updated the following, -> Removed repr,

• For service class, as we are resolving the service and throwing error if it's invalid.
• For class name, as we are validating the class name with isidentifier.

-> Updated the display name with repr and stripped the quotes, as we are retrieving the display name from metadata.