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

Add Type Confusion Requirement to ASVS #1617

Closed
ImanSharaf opened this issue Apr 18, 2023 · 12 comments · Fixed by #1780
Closed

Add Type Confusion Requirement to ASVS #1617

ImanSharaf opened this issue Apr 18, 2023 · 12 comments · Fixed by #1780
Assignees
Labels
2) Awaiting response Awaiting a response from the original poster Community wanted We would like feedback from the community to guide our decision otherwise we will progress _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@ImanSharaf
Copy link
Collaborator

I noticed that there is a missing requirement related to Type Confusion. Type confusion is not the same as type juggling which we want to add to the ASVS. In type juggling automatic conversion of data types is the root cause of the problem where in the type confusion incorrectly interpreting data types is the root cause.
While in languages such as C/C++ this leads to high severity vulnerabilities such as CVE-2023-0286 (OpenSSL), in JavaScript language it leads to some serious vulnerabilities too. For example, in this example by Snyk, it leads to sanitization bypass and XSS.
A sample ASVS could be this one:

Ensure that the application handles type confusion by using strict equality operators, validating input types, and being cautious with shared method behavior on different types

To decode the check (for JavaScript language):

  • Use strict equality (===) and inequality (!==) operators to compare values, as they do not perform type coercion, which can lead to unexpected results.
  • Be cautious when using property accessors with bracket notation, as non-string keys will be coerced to strings before accessing the object property.
  • When using built-in methods like indexOf() or includes(), which are defined on both String and Array types, be aware that they may behave differently depending on the input type. Perform type checks before using these methods for input validation or sanitization.
@tghosth tghosth added 2) Awaiting response Awaiting a response from the original poster _5.0 - prep This needs to be addressed to prepare 5.0 labels Jul 10, 2023
@tghosth
Copy link
Collaborator

tghosth commented Jul 10, 2023

Hi @ImanSharaf, I have to say that I think that type confusion and type juggling (as discussed in #1539) seem very similar to me as they both stem from the language making an assumption or inference about the type of a variable rather than the type being explicitly stated.

As such, I would be inclined to suggest a combined requirement such as:

Verify that the application explicitly ensures that variables are of the correct type and performs strict equality and comparator operations to avoid type juggling or confusion vulnerabilities caused by the application code making an assumption about a variable type.

What do you think?

@tghosth
Copy link
Collaborator

tghosth commented Jul 10, 2023

Note to self, there is a discussion in #1539 as to what section this requirement should go into.

@tghosth
Copy link
Collaborator

tghosth commented Sep 21, 2023

Hi @ImanSharaf, I have to say that I think that type confusion and type juggling (as discussed in #1539) seem very similar to me as they both stem from the language making an assumption or inference about the type of a variable rather than the type being explicitly stated.

As such, I would be inclined to suggest a combined requirement such as:

Verify that the application explicitly ensures that variables are of the correct type and performs strict equality and comparator operations to avoid type juggling or confusion vulnerabilities caused by the application code making an assumption about a variable type.

What do you think?

@ImanSharaf what do you think about this wording?

@ImanSharaf
Copy link
Collaborator Author

Hi @ImanSharaf, I have to say that I think that type confusion and type juggling (as discussed in #1539) seem very similar to me as they both stem from the language making an assumption or inference about the type of a variable rather than the type being explicitly stated.
As such, I would be inclined to suggest a combined requirement such as:

Verify that the application explicitly ensures that variables are of the correct type and performs strict equality and comparator operations to avoid type juggling or confusion vulnerabilities caused by the application code making an assumption about a variable type.

What do you think?

@ImanSharaf what do you think about this wording?

This appears to be satisfactory. @tghosth

@tghosth
Copy link
Collaborator

tghosth commented Sep 26, 2023

@elarlang what do you think about putting this into 5.4 and renaming the section to: "Memory Safety and Defensive Coding".

It is a little ambiguous but I am not sure we can escape that. Otherwise, what do you suggest? (Note my point in #1731 as well).

@elarlang
Copy link
Collaborator

elarlang commented Sep 26, 2023

what do you think about putting this into 5.4 and renaming the section to: "Memory Safety and Defensive Coding".

it is like creating new meal from leftovers...

It is a little ambiguous but I am not sure we can escape that. Otherwise, what do you suggest? (Note my point in #1731 as well).

Maybe it makes sense to combine some "code review" requirements from current V10 (related opened issues #1383, #1468).

@jmanico
Copy link
Member

jmanico commented Sep 26, 2023 via email

@tghosth
Copy link
Collaborator

tghosth commented Sep 27, 2023

I love leftovers and I'm about to have them for lunch

I don't think this is a malicious code issue so I don't really want to categorise it like that. I am going to add community wanted and let this percolate a bit.

@tghosth tghosth added the Community wanted We would like feedback from the community to guide our decision otherwise we will progress label Sep 27, 2023
@tghosth
Copy link
Collaborator

tghosth commented Nov 8, 2023

I don't think this is a malicious code issue so I don't really want to categorise it like that. I am going to add community wanted and let this percolate a bit.

@elarlang @jmanico alternative suggestion, we rename chapter V10 to Insecure and Malicious Code and include a Defensive Coding section in this chapter.

What do you think?

(Note to self, opened branch: https://github.dev/OWASP/ASVS/tree/defensive_coding)

@elarlang
Copy link
Collaborator

elarlang commented Nov 8, 2023

V10 will be quite empty after clean-up so it makes sense to make it more abstract, maybe covering all "code-review" related issues which do not belong clearly somewhere else.

@jmanico
Copy link
Member

jmanico commented Nov 14, 2023

I like the idea of a code review / secure design section for developers. A lot.

@tghosth
Copy link
Collaborator

tghosth commented Nov 15, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2) Awaiting response Awaiting a response from the original poster Community wanted We would like feedback from the community to guide our decision otherwise we will progress _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants