Skip to content
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

Data from one API to another API of the SDK needs Sanitization and Validation #2239

Open
ericwol-msft opened this issue Jun 16, 2022 · 2 comments
Assignees
Labels
Azure.Core bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library.

Comments

@ericwol-msft
Copy link
Contributor

Improper Input Validation (CWE-20) inside Trust Boundary:
• Data from one API to another API of the SDK needs Sanitization and Validation, for future safety concerns.

All internal APIs should be reviewed for proper input validation

• File : src\azure\core\az_json_writer.c
• API : _az_validate_json( )
• Lines : 717 – 750
• Issue : Improper Input Validation is Missing for:
• json_text
• last_token_kind
• Caller Graph:
image

image

@ghost ghost added the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Jun 16, 2022
@RickWinter RickWinter added bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. Azure.Core labels Jun 20, 2022
@ghost ghost removed the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Jun 20, 2022
@ahsonkhan
Copy link
Member

ahsonkhan commented Jun 21, 2022

There is only one caller of this internal method, and it is guaranteed to call it correctly, but we should add a null precondition check on last_token_kind for maintainability and consistency.

The need for json_text validation is interesting. The JSON reader init method does the precondition validation already. Is the ask to duplicate such validation? That's doable, but it seems unnecessary, and looks like a restriction of the detection tool.

Is there a way to opt-out of specific instances of false positives, based on context and code inspection?

@ahsonkhan
Copy link
Member

The need for json_text validation is interesting. The JSON reader init method does the precondition validation already. Is the ask to duplicate such validation? That's doable, but it seems unnecessary, and looks like a restriction of the detection tool.

After discussing with @ericwol-msft, this is discovered through manual code inspection, so regarding json_text we landed on adding a comment stating that the reader init method does the appropriate validation, and not to duplicate the precondition, unnecessarily.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

No branches or pull requests

4 participants