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

5.2.2 sounds like input validation #1640

Closed
tghosth opened this issue May 30, 2023 · 13 comments
Closed

5.2.2 sounds like input validation #1640

tghosth opened this issue May 30, 2023 · 13 comments
Assignees
Labels
7) PR in non-master branch V5 Temporary label for grouping input validation, sanitization, encoding, escaping related requirements _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@tghosth
Copy link
Collaborator

tghosth commented May 30, 2023

This requirement is in the sanitization section but sounds like input validation. The CWE also doesn't make sense to me.

# Description L1 L2 L3 CWE
5.2.2 Verify that unstructured data is sanitized to enforce safety measures such as allowed characters and length. 138
@tghosth tghosth added _5.0 - prep This needs to be addressed to prepare 5.0 4a) Waiting for another This issue is waiting for another issue to be resolved V5 Temporary label for grouping input validation, sanitization, encoding, escaping related requirements labels May 30, 2023
@elarlang
Copy link
Collaborator

elarlang commented May 30, 2023

edit: Maybe should be solved here: #1552

@elarlang
Copy link
Collaborator

elarlang commented May 30, 2023

Actually, it's valid requirement. Point for sanitization is, if input validation can not be used, you need to send valid and safe input to next component (downstream) and therefore you need to be sure, that it only contains allowed characters and is with expected/safe length.

@tghosth tghosth added 4b Major-rework These issues need to be part of a full chapter rework and removed 4a) Waiting for another This issue is waiting for another issue to be resolved labels May 30, 2023
@elarlang
Copy link
Collaborator

elarlang commented Feb 6, 2024

Is there any certain problem to discuss or is there a need for improvement? I think it is suitable for "covering all sanitization cases" which does not have separate requirements per situation.

ping @tghosth

@elarlang elarlang added the 2) Awaiting response Awaiting a response from the original poster label Feb 6, 2024
@tghosth
Copy link
Collaborator Author

tghosth commented Feb 6, 2024

You think it is like a catch-all or something? It does seem a little vague...

@elarlang elarlang added the next meeting Filter for leaders label Feb 8, 2024
@tghosth
Copy link
Collaborator Author

tghosth commented Feb 15, 2024

Verify that unstructured data is sanitized to before being used in any other potentially dangerous context not already listed in this section.

Thought about this but we don't love the fact that it references other requirements.

On the other hand, not sure how to accomplish this as a "last resort/catch-all requirement" without doing so.

@tghosth
Copy link
Collaborator Author

tghosth commented Feb 15, 2024

Need to think more about it.

@tghosth tghosth removed the next meeting Filter for leaders label Mar 21, 2024
@tghosth
Copy link
Collaborator Author

tghosth commented Aug 12, 2024

I now think this is a duplicate of 5.1.3.

I think we should do as follows:

# Description L1 L2 L3 CWE
5.1.3 [MODIFIED, MERGED FROM 5.2.2] Verify that all input, including unstructured data, is validated using positive validation, with an allowed list of characters, values or patterns, and an appropriate maximum length. 20
# Description L1 L2 L3 CWE
5.2.2 [DELETED, MERGED TO 5.1.3]

What do you think @elarlang ?

@elarlang
Copy link
Collaborator

I stand for my comment in #1640 (comment)

Actually, it's valid requirement. Point for sanitization is, if input validation can not be used, you need to send valid and safe input to next component (downstream) and therefore you need to be sure, that it only contains allowed characters and is with expected/safe length.

For me the requirement must stay and I'm ok with the requirement as it is. Also, I have pointed and quoted to this requirement many times as the reason to not have some other requirement. So it's clear no for me to remove it.

@tghosth
Copy link
Collaborator Author

tghosth commented Aug 15, 2024

Ok so in that case maybe this clashes with the new 5.3.14 and we need to merge them @elarlang?

# Description L1 L2 L3 CWE
5.3.14 [ADDED] Verify that output encoding is relevant for the interpreter and context required in any context where a potentially dangerous interpreter, not mentioned above, is being used.

@elarlang elarlang added the next meeting Filter for leaders label Aug 15, 2024
@tghosth
Copy link
Collaborator Author

tghosth commented Aug 15, 2024

This requirement is important for a situation where output encoding is not possible.

We should be clear that sanitization is required when there is no standard encoding or escaping that can be performed. Maybe in the intro text for the section.

Suggested update discussed with @elarlang:

# Description L1 L2 L3 CWE
5.2.2 [MODIFIED] Verify that data being passed to a potentially dangerous context is sanitized beforehand to enforce safety measures, such as only allowing characters which are safe for this context and trimming input which is too long. 138

@set-reminder 7 days @tghosth to open a PR and also check intro text.

Copy link

octo-reminder bot commented Aug 15, 2024

Reminder
Thursday, August 22, 2024 12:00 AM (GMT+02:00)

@tghosth to open a PR and also check intro text.

@tghosth tghosth removed the next meeting Filter for leaders label Aug 15, 2024
Copy link

octo-reminder bot commented Aug 21, 2024

🔔 @tghosth

@tghosth to open a PR and also check intro text.

@tghosth
Copy link
Collaborator Author

tghosth commented Aug 25, 2024

Fix to requirement in #2028 (@elarlang to review)

(Change to intro text will be tracked via #2027 and updated separately)

tghosth added a commit that referenced this issue Aug 26, 2024
@tghosth tghosth added 7) PR in non-master branch _5.0 - prep This needs to be addressed to prepare 5.0 and removed _5.0 - prep This needs to be addressed to prepare 5.0 2) Awaiting response Awaiting a response from the original poster 4b Major-rework These issues need to be part of a full chapter rework labels Sep 2, 2024
@tghosth tghosth closed this as completed in ab5bd6b Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7) PR in non-master branch V5 Temporary label for grouping input validation, sanitization, encoding, escaping related requirements _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

2 participants