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

Update Input Validation #1429

Merged
merged 2 commits into from
Jun 17, 2024
Merged

Update Input Validation #1429

merged 2 commits into from
Jun 17, 2024

Conversation

otkd
Copy link
Contributor

@otkd otkd commented Jun 16, 2024

  • Clarify Server-side & Client-side recommendations
  • Add references
  • Style changes for uniformity

Thank you for submitting a Pull Request (PR) to the Cheat Sheet Series.

🚩 If your PR is related to grammar/typo mistakes, please double-check the file for other mistakes in order to fix all the issues in the current cheat sheet.

Please make sure that for your contribution:

  • In case of a new Cheat Sheet, you have used the Cheat Sheet template.
  • All the markdown files do not raise any validation policy violation, see the policy.
  • All the markdown files follow these format rules.
  • All your assets are stored in the assets folder.
  • All the images used are in the PNG format.
  • Any references to websites have been formatted as [TEXT](URL)
  • You verified/tested the effectiveness of your contribution (e.g., the defensive code proposed is really an effective remediation? Please verify it works!).
  • The CI build of your PR pass, see the build status here.

If your PR is related to an issue, please finish your PR text with the following line:

This PR covers issue #.

Thank you again for your contribution 😃

- Clarify Server-side & Client-side recommendations
- Add references
- Style changes for uniformity

Signed-off-by: otkd <conrad.bhuiyanvolkoff@owasp.org>
@otkd otkd requested review from mackowski, jmanico and szh as code owners June 16, 2024 18:59
szh
szh previously approved these changes Jun 16, 2024
cheatsheets/Input_Validation_Cheat_Sheet.md Outdated Show resolved Hide resolved
Co-authored-by: Shlomo Zalman Heigh <shlomozalmanheigh@gmail.com>
Copy link
Collaborator

@kwwall kwwall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My $.02: "Whitelist" was in the dictionary (e.g., see https://dictionary.cambridge.org/us/dictionary/english/whitelist). But while "allowlist" is it a lot of technical glossaries and is broadly used, I haven't yet found it in any prominent dictionary of the English language (of any variety). So, until it is--or if you can convince the Cambridge Dictionary, the American Heritage Dictionary, etc. to add it--I think we should continue to write it out as 2 hyphenated words, e.g., "allow-list". (Same with "block-list" if you changed that to "blocklist".) Other than that, LGTM.

@otkd
Copy link
Contributor Author

otkd commented Jun 16, 2024

My $.02: "Whitelist" was in the dictionary (e.g., see https://dictionary.cambridge.org/us/dictionary/english/whitelist). But while "allowlist" is it a lot of technical glossaries and is broadly used, I haven't yet found it in any prominent dictionary of the English language (of any variety). So, until it is--or if you can convince the Cambridge Dictionary, the American Heritage Dictionary, etc. to add it--I think we should continue to write it out as 2 hyphenated words, e.g., "allow-list". (Same with "block-list" if you changed that to "blocklist".) Other than that, LGTM.

This perhaps would be a reasonble approach if all terminology used by OWASP would be in general purpose dictionaries, however given the intended audience this is unlikely to be the case.

"Allowlist" is for example recommended by:

@otkd otkd requested a review from kwwall June 16, 2024 23:37
Copy link
Collaborator

@kwwall kwwall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a showstopper. I'm not going to allow the perfect to become the enemy of the good here. But it wouldn't surprise me to see someone create an issue and a PR later on claiming it's not in the dictionary. (Although, that someone will not be me.)

@@ -54,8 +53,8 @@ It's also free-form text input that highlights the importance of proper context-
The primary means of input validation for free-form text input should be:

- **Normalization:** Ensure canonical encoding is used across all the text and no invalid characters are present.
- **Character category allow-listing:** Unicode allows listing categories such as "decimal digits" or "letters" which not only covers the Latin alphabet but also various other scripts used globally (e.g. Arabic, Cyrillic, CJK ideographs etc).
- **Individual character allow-listing:** If you allow letters and ideographs in names and also want to allow apostrophe `'` for Irish names, but don't want to allow the whole punctuation category.
- **Character category allowlisting:** Unicode allows listing categories such as "decimal digits" or "letters" which not only covers the Latin alphabet but also various other scripts used globally (e.g. Arabic, Cyrillic, CJK ideographs etc).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #1429 (comment), @otkd wrote:

This perhaps would be a reasonble approach if all terminology used by OWASP would be in general purpose dictionaries, however given the intended audience this is unlikely to be the case.

"Allowlist" is for example recommended by:

NIST
Inclusive Naming Initiative members: Cisco, Intel, Red Hat, Canonical, CNCF, Extreme Networks etc.
Google
Linux Kernel

My concern is not that the audience of the CS will fail to understand it. It's more of a concern that until it becomes part of the official dictionaries, it's going to get flagged by spell-checkers, which is just annoying as hell. (For instance, if Firefox in US_en, "allowlist" has the little red squiggle, despite me adding to a custom dictionary numerous times.)

@jmanico
Copy link
Member

jmanico commented Jun 17, 2024

"allowlist" is the NIST standard. Dictionaries be dammed. Let's go with allowlist.

Other dictionaries that support this:

@jmanico jmanico merged commit a289f52 into OWASP:master Jun 17, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants