Cheat sheet update/refactor proposal: [Insecure_Direct_Object_Reference_Prevention_Cheat_Sheet]

[GrosQuildu]

1. What is missing or needs to be updated in the current iteration of the cheat sheet? Two things in the proposed IDOR prevention code seems to be worth :

• sha1(salt+message) is vulnerable to length extension. Hardly exploitable, but why give bad patterns in security-minded code?
• Computing hash for every movie object in every request is, hm.. unefficient?

2. How do you intend to resolve this with an update/refactoring?

• We should use hmac or something, don't we?
• At least mention that it should be rewritten with database/memory cache, so people won't happily copy-paste it?

[mackowski]

Hey @GrosQuildu thank you for this issue. @righettod I believe that you are the author :-) What is your opinion?

[righettod]

Hello

sha1(salt+message) is vulnerable to length extension. Hardly exploitable, but why give bad patterns in security-minded code?

The goal is just to prevent the disclosure of the real identifier (prevention of a pattern in the ID and via the SALT you cannot reproduce a ID if you do not know the SALT), it's very important to ensure that the ACL on the resource are well implemented, IDOR is often a booster to spot ACL issue. As mentioned in the code, if your system support >= SHA256 so feel free to use it. It's also true that you can use HMAC. I have just proposed a simple way to hide the real identifer using an algorithm performing quick computing in a way that the info to hide is not sensitive so it's why I do not have used HMAC.

Computing hash for every movie object in every request is, hm.. unefficient?

100% true but it depend if you want/need to be stateful or stateless. If you can add a technical ID (non guessage/iterable like UUID) into the storage of the resource and use it then it's really a better solution. However, when you cannot add this new technical identifier, you need to compute it on-the-fly and either keep the mapping in the session (statefull) or compute it in the query (stateless).

It's true that the current CS miss the description provided above and need an update to define the different context in which IDOR protection can occur.

[mackowski]

Thanks Dom! I agree. @GrosQuildu do you want to prepare PR where you add more detailed description and other ways how to implement it? You can also add some pros and cons of different solutions.

[aiacobelli2]

Hi to all, just a comment here.

I agree that the best way to "resolve" the direct identifier is to use an inline transformation that creates a public ("RANDOM") value but I do not know if hash is the best approach, I would recommend any CRPRNG based construction like UUIDV4. Why? Using a hash approach (h(value+salt)), you need to generate a random SALT and store it somewhere, so you need to generate a pseudo random sequence anyways, adding a hash to the formula do not give extra security.

It's best to add just a key value store of File -> PseudoRandomGeneratedString and depending on the threat model, decide if you need to be stateless or stateful

What do you think?

[ThunderSon]

@ankane guess this is something that could assist. Let's discuss this on Slack and then write down our findings on this to see this forward 😄

[jmanico]

I'd like to see this entire cheatsheet rewritten. IDOR is an access control issue that should be solved with lookup maps, data ownership verification, and other mature horizontal access control designs.

[mackowski]

@righettod do you want to contribute agin :)

[jmanico]

Perhaps @GrosQuildu cares to work on this?

[Sjord]

Rewrite IDOR cheat sheet by Sjord · Pull Request #1177 · OWASP/CheatSheetSeries