[Clarification/For Discussion] What does 5.5.1 control prevent?

[csfreak92]

ASVS 4.0 - 5.5.1 verification requirement is:

Verify that serialized objects use integrity checks or are encrypted to prevent hostile object creation or data tampering.

Is this control talking about link level encryption or object/payload level encryption? Is it protecting the application from a man-in the browser attack (link level encryption) or preventing a user from modifying his data (object/payload level encryption)? If those are the case, then I think clarifying the main point of this control would help developers know what to do to protect their application from such attacks and I would propose a clearer language in a pull request like:

Verify that serialized objects use integrity checks (object/payload level encryption) or are encrypted (link level encryption) to prevent hostile object creation or data tampering.

[tghosth]

My understand of this control is that it is specifically refering to the serialized object. Either an integrity check (like a hash or signature) or encryption should be used to ensure it has not been tampered with.

Not sure how else to clarify that. Any thoughts?

[csfreak92]

Aren't hashes or signatures of input values coming from the client-side prone to tampering as well? If a serialized object (with constant values) are being round-tripped from the server to the client and back to the server, then hashes or signatures could work. But if the input is fresh from the client and to the server, then those won't work. Am I missing something?

Encryption would be the best, although I think link level encryption could suffice for most applications, object level encryption would be a better recommendation for applications with higher risks. Should we mention this somewhere in the controls in ASVS?

[jmanico]

A few notes, Digital signatures are NOT prone to tampering, they provide integrity, the whole point of digital signatures in standards like JWT's is to detect and avoid tampering. Encryption */does not always provide integrity/* (ie tamper resistance) unless it's a newer mode. Encryption does not stop deserialization attacks, traditionally. In Java you would need to use the serialization filter API's and design your objects in a special way per https://www.oracle.com/java/technologies/javase/seccodeguide.html . For .NET you need to address https://www.youtube.com/watch?v=oxlD8VWWHE8 I have not seen crypto as a solution to the deserialization problem in conventional secure coding wisdom... - Jim

On 5/21/20 5:06 PM, csfreak92 wrote: Aren't hashes or signatures of input values coming from the client-side prone to tampering as well? If a serialized object (with constant values) are being round-tripped from the server to the client and back to the server, then hashes or signatures could work. But if the input is fresh from the client and to the server, then those won't work. Am I missing something? Encryption would be the best, although I think link level encryption could suffice for most applications, object level encryption would be a better recommendation for applications with higher risks. Should we mention this somewhere in the controls in ASVS? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#775 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEBYCM5KINHN6ZYZO536UTRSWJVHANCNFSM4NG6YWSA>.

-- Jim Manico Manicode Security https://www.manicode.com

[csfreak92]

Sorry, let me rephrase what I meant by my previous comment. I might have been confusing.

Aren't hashes or signatures of input values coming from the client-side prone to tampering as well? If a serialized object (with constant values) are being round-tripped from the server to the client and back to the server, then hashes or signatures could work. But if the input is fresh from the client and to the server, then those won't work. Am I missing something?

What I meant here was if we will generate signatures or hashes from the client-side every time new input is received, then that is prone to tampering and it could be circumvented. If the hashes or signatures are generated on client-side which would be a horrible implementation of it. However, if hashes or signatures are done on the server-side and the values are being passed around then it helps. But what about new inputs? I think that's the problematic part in serialization, receiving input that could end up causing deserialization issues, no?

Encryption would be the best, although I think link level encryption could suffice for most applications, object level encryption would be a better recommendation for applications with higher risks. Should we mention this somewhere in the controls in ASVS?

What I meant here was if we have link level encryption (communications encryption) then it could help but still be bypassed and I totally agree with you on that, so object/payload level encryption would be best.

@jmanico, isn't encryption on a payload level providing tamper resistance?

[jmanico]

No, symmetric encryption does not provide integrity. Only modern symmetric encryption modes that are combined with integrity checks do that; you don’t get it automatically get integrity when using symmetric encryption. http://en.wikipedia.org/wiki/Authenticated_encryption

…

-- Jim Manico @manicode Secure Coding Education +1 (808) 652-3805

On Jun 1, 2020, at 4:51 PM, csfreak92 ***@***.***> wrote:  Sorry, let me rephrase what I meant by my previous comment. I might have been confusing. Aren't hashes or signatures of input values coming from the client-side prone to tampering as well? If a serialized object (with constant values) are being round-tripped from the server to the client and back to the server, then hashes or signatures could work. But if the input is fresh from the client and to the server, then those won't work. Am I missing something? What I meant here was if we will generate signatures or hashes from the client-side every time new input is received, then that is prone to tampering and it could be circumvented. If the hashes or signatures are generated on client-side which would be a horrible implementation of it. However, if hashes or signatures are done on the server-side and the values are being passed around then it helps. But what about new inputs? I think that's the problematic part in serialization, receiving input that could end up causing deserialization issues, no? Encryption would be the best, although I think link level encryption could suffice for most applications, object level encryption would be a better recommendation for applications with higher risks. Should we mention this somewhere in the controls in ASVS? What I meant here was if we have link level encryption (communications encryption) then it could help but still be bypassed and I totally agree with you on that, so object/payload level encryption would be best. @jmanico, isn't encryption on a payload level providing tamper resistance? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[elarlang]

@csfreak92 - for "new input" there is another requirement:

V5.5.3 Verify that deserialization of untrusted data is avoided or is protected in both custom code and third-party libraries (such as JSON, XML and YAML parsers).

But V5.5.1 is not clear for me also. Isn't it duplicate for V5.5.3 " or is protected in both custom code and third-party libraries (such as JSON, XML and YAML parsers)"? What is this protection?

Does it makes it more clear?

V5.5.1 Verify that serialized objects use integrity checks on/for/at unserialize or are encrypted to prevent hostile object creation or data tampering.

To validate requirements - where to use "PHP Object Injection" - input is generated by applcation, sent to user, and userinput goes back directly to unserialize function.

From V5.5.1 point of view - integrity check is missing.
From V5.5.3 point of view - an applcation should not take user input to unserialize function at all.

My question/point, do V5.5.1 and V5.5.3 actually address the same problem and should me merged?

[jmanico]

For 5.5.3 don’t we have other standard requirements to keep libraries up to date? This is likely a duplicate. To stop deserialization you should filter incoming deserialIzation data with language features like JEP 290 in Java.

…

-- Jim Manico @manicode Secure Coding Education +1 (808) 652-3805

On Jun 10, 2020, at 3:04 PM, Elar Lang ***@***.***> wrote:  @csfreak92 - for "new input" there is another requirement: V5.5.3 Verify that deserialization of untrusted data is avoided or is protected in both custom code and third-party libraries (such as JSON, XML and YAML parsers). But V5.5.1 is not clear for me also. Isn't it duplicate for V5.5.3 " or is protected in both custom code and third-party libraries (such as JSON, XML and YAML parsers)"? What is this protection? Does it makes it more clear? V5.5.1 Verify that serialized objects use integrity checks on/for/at unserialize or are encrypted to prevent hostile object creation or data tampering. To validate requirements - where to use "PHP Object Injection" - input is generated by applcation, sent to user, and userinput goes back directly to unserialize function. From V5.5.1 point of view - integrity check is missing. From V5.5.3 point of view - an applcation should not take user input to unserialize function at all. My question/point, do V5.5.1 and V5.5.3 actually address the same problem and should me merged? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[elarlang]

For "up to date" there is requirement:

V14.2.1 Verify that all components are up to date, preferably using a dependency checker during build or compile time.

Personally I don't see/feel relation between up-to-date requirement and current V5.3.3

[jmanico]

They are the exact same requirement. 5.3.3 says to keep your JSON and XML parsers up to date which is a subset of the part of 14.2.1 before the first comma. - Jim

…

On 6/10/20 4:40 PM, Elar Lang wrote: For "up to date" there is requirement: V14.2.1 Verify that all components are up to date, preferably using a dependency checker during build or compile time. Personally I don't see/feel relation between up-to-date requirement and current V5.3.3 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#775 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEBYCNITNKKQE3MQ26MGGDRV7VVTANCNFSM4NG6YWSA>.

[elarlang]

V5.5.3 Verify that deserialization of untrusted data is avoided or is protected in both custom code and third-party libraries (such as JSON, XML and YAML parsers).

V14.2.1 Verify that all components are up to date, preferably using a dependency checker during build or compile time.

From:

V5.5.3 Verify that deserialization of untrusted data is avoided or is protected in both custom code and third-party libraries (such as JSON, XML and YAML parsers).

I interpret it as:

V5.5.3 Verify that deserialization of untrusted data is protected in third-party libraries (such as JSON, XML and YAML parsers).

"data is protected by third party libraries" vs "third party libraries are up to date" - if they are the same thing, then it makes sense to remove second part of V5.3.3 and merge first part of V5.3.3 with V5.3.1.

[jmanico]

To be more clear, 5.5.3 is *in part* overlapping with the 14.x requirement. But I still don't like 5.5.3. at all, its' not actionable over than the 14x overlap and think it needs to go. - Jim

On 6/10/20 4:57 PM, Elar Lang wrote: V5.5.3 Verify that deserialization of untrusted data is avoided or is protected in both custom code and third-party libraries (such as JSON, XML and YAML parsers). V14.2.1 Verify that all components are up to date, preferably using a dependency checker during build or compile time. From: V5.5.3 *Verify that deserialization of untrusted data* is avoided or *is protected in* both custom code and *third-party libraries (such as JSON, XML and YAML parsers)*. I interpret it as: V5.5.3 Verify that deserialization of untrusted data is protected in third-party libraries (such as JSON, XML and YAML parsers). "data is protected by third party libraries" vs "third party libraries are up to date" - if they are the same thing, then it makes sense to remove second part of V5.3.3 and merge first part of V5.3.3 with V5.3.1. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#775 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEBYCOQ6JLDGYQTRLW5EYTRV7XVFANCNFSM4NG6YWSA>.

-- Jim Manico Manicode Security https://www.manicode.com

[elarlang]

I made separate issue for removing V5.5.3.

Back to V5.5.1 - if V5.5.3 will be removed, V5.5.1 should also cover "left-overs" from V5.5.3.

Current:

V5.5.1 Verify that serialized objects use integrity checks or are encrypted to prevent hostile object creation or data tampering.

It should cover now also:

• "deserialization of untrusted data is avoided"

with my estonglish (estonian version of english) I don't even try to propose anything...

[jmanico]

5.5.1 makes no sense. Encryption and integrity checks are not (at all) the right solution to address deserialization problems. Filtering incoming deserialization data (for Java see JEP 290) and proper object design are needed to safely deserialize untrusted binary data. - Jim

…

On 6/10/20 5:12 PM, Elar Lang wrote: I made separate issue for removing V5.5.3. Back to V5.5.1 - if V5.5.3 will be removed, V5.5.1 should also cover "left-overs" from V5.5.3. Current: V5.5.1 Verify that serialized objects use integrity checks or are encrypted to prevent hostile object creation or data tampering. It should cover now also: * "deserialization of untrusted data is avoided" with my estonglish (estonian version of english) I don't even try to propose anything... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#775 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEBYCIZHUS6IXIFTENRADDRV7ZM3ANCNFSM4NG6YWSA>.

[elarlang]

If you send (for some reason) serialized data from one service to another (or back to the same service), it makes sense for me to use hmac - if it does not match, no action.

[jmanico]

So you are saying that the initial binary data set data could not be tainted by user input? So it's all "trusted" data? Then why the HMAC? That not likely, I likely can taint that binary blob with user input before you HMAC. The HMAC only provides transport integrity - which I get via TLS anyways. The only time to HMAC a payload in API security actually helps security is when you are throwing JWT's around and do not what them tainted - and JWT's contain zero user data. So HMAC'ing intra-service calls is likely zero net benefit over well configured TLS. You're confusing intra-service security with deserialization security. This is not a good requirement or deserialization and confuses the issue and it should go away. - Jim

On 6/10/20 5:22 PM, Elar Lang wrote: If you send (for some reason) serialized data from one service to another (or back to the same service), it makes sense for me to use hmac - if it does not match, no action. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#775 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEBYCPRKDBKU22J3XKHZ7DRV72RNANCNFSM4NG6YWSA>.

-- Jim Manico Manicode Security https://www.manicode.com

[elarlang]

If you send (for some reason) serialized data from one service to another (or back to the same service), it makes sense for me to use hmac - if it does not match, no action.

hmac here is only for integrity check, I don't touch confidentiality here.

[jmanico]

You are using the wrong crypto primitive for intra-service communication.  TLS gives you integrity in transit. And if untrusted data feeds your intra-service request (which is almost a definite) the attacker can taint the payload before you HMAC and send that data intra-service, defeating the benefit. The only time I see message-level HMAC'ing as an actual useful security control, is when the server hands the client a token or other data that the client is not allowed to modify in any way. (ala JWT) but if your users are submitting untrusted input that you use to build a payload that you then HMAC (a common use case) then you're building a control that does not provide any benefit. And if you are using JWT I suggest you avoid HMAC's and use pub/pri key for integrity in a microsservice environment so you have less of a private key distribution burden.  -Jim

On 6/11/20 3:45 AM, Elar Lang wrote: If you send (for some reason) serialized data from one service to another (or back to the same service), it makes sense for me to use hmac - if it does not match, no action. hmac here is only for integrity check, I don't touch confidentiality here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#775 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEBYCNQEPZQFNXCFJUUYDDRWCDRBANCNFSM4NG6YWSA>.

-- Jim Manico Manicode Security https://www.manicode.com

[elarlang]

Any changes to the current requirement V5.5.1 for ASVS v4.0.2 or you want to re-create entire topic for ASVS v4.1 and issue #807 ?

[jmanico]

5.5.1 is incorrect, I state with respect. I would like it to go away as soon as possible. Untrusted data -> Webserver where untrusted data is added to binary data -> HMAC AND OR ENCRYPTION ON TOP OF TLS  AS PAYLOAD IS SENT TO API -> Deserialization happens with partial untrusted data -> KaBoom - Jim

On 6/11/20 10:19 AM, Elar Lang wrote: Any changes to the current requirement V5.5.1 for ASVS v4.0.2 or you want to re-create entire topic for ASVS v4.1 and issue #807 <#807> ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#775 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEBYCIZD274FD72TVCRLOTRWDRVZANCNFSM4NG6YWSA>.

-- Jim Manico Manicode Security https://www.manicode.com

[csfreak92]

@elarlang, I agree with @jmanico with this. I think 5.1.1 is too problematic as a requirement. It seems that 5.1.3 covers deserialization issues anyway and having 5.1.1 just confuses a tester/developer. I believe our discussion about this was helpful in determining whether integrity checks really help in preventing hostile object tampering or creation. I vote to remove 5.1.1 as well unless there's something else we are missing to cover for integrity checks.

[elarlang]

So, to conclude and be clear:
remove current V5.5.1 from ASVS v4.1 (and no changes to ASVS v4.0.2)?

[jmanico]

Remove from both please.

…

-- Jim Manico @manicode

On Jun 11, 2020, at 1:33 PM, Elar Lang ***@***.***> wrote:  So, to conclude and be clear: remove current V5.5.1 from ASVS v4.1 (and no changes to ASVS v4.0.2)? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[elarlang]

I understood that we will not have any removals for v4.0.2 (if not clear duplicate).

#750 (comment)

[jmanico]

Josh?

…

-- Jim Manico @manicode

On Jun 11, 2020, at 4:29 PM, Elar Lang ***@***.***> wrote:  I understood that we will not have any removals for v4.0.2 (if not clear duplicate). #750 (comment) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[tghosth]

Thanks @elarlang, I agree that this sounds too major a change for 4.0.2 but should be changed for 4.1.

[tghosth]

To quote @vanderaj:

Branch 4.0.2 - bug fixes only. Will require some work to back port the
fixes from master. The only thing I would countenance here is duplicate
issue removal, spelling fixes, and clarifications.

[vanderaj]

I will review this issue carefully for 4.0.2, re-wording is okay, but additions, substantial changes of meaning, etc. need to be 4.1.

[jmanico]

Can we just drop 5.5.1 from 4.0.2? It's off base.

[jmanico]

requirement removed per 1180694

[elarlang]

Reopen the issue for:

• @tghosth rechecking related issues and changes
• posting my label validation / mapping between versions just in case we need to revisit it

v4.0.3

Related requirements:

#	Description	L1	L2	L3	CWE
1.5.2	Verify that serialization is not used when communicating with untrusted clients. If this is not possible, ensure that adequate integrity controls (and possibly encryption if sensitive data is sent) are enforced to prevent deserialization attacks including object injection.		✓	✓	502
5.5.1	Verify that serialized objects use integrity checks or are encrypted to prevent hostile object creation or data tampering. (C5)	✓	✓	✓	502
5.5.3	Verify that deserialization of untrusted data is avoided or is protected in both custom code and third-party libraries (such as JSON, XML and YAML parsers).	✓	✓	✓	502

Related issues:

• [Clarification/For Discussion] Multiple Clarifications for 1.5.2 #772
• [Clarification/For Discussion] What does 5.5.1 control prevent? #775
• remove V5.5.3, it's is duplicate of V14.2.1 and not actionable #807
• duplicates: 1.5.2 and 5.5.3 #1084

v5.0 as it stands

#	Description	L1	L2	L3	CWE
1.5.2	[DELETED, MERGED TO 5.5.3]
5.5.1	[DELETED]
5.5.3	[MODIFIED, MERGED FROM 1.5.2] Verify that deserialization is not used when communicating with untrusted clients. If this is not possible, ensure that deserialization is performed safely, for example, by only allowing a allow-list of object types or not allowing the client to define the object type to deserialize to, in order to prevent deserialization attacks.	✓	✓	✓	502

I try to get rid of just "DELETED" labels and the question is - the most suitable reason for 5.5.1 seems to be DELETED, INCORRECT. No need to reference to any requirement, like 5.5.3. In case we need, then it should be MERGED TO 5.5.3 (which is not valid) or DEPRECATED BY 5.5.3

[tghosth]

I would support DELETED, INCORRECT