Handover Gray Box and Code Reviews to SKF

[ThunderSon]

What would you like to happen? Having a Code Review section, where a tester can communicate with the developers and have direct access to the source code is more descriptive than Gray Box, because actually having access to the source code is white box to begin with.

What do you think?

[kingthorin]

I'd rather leave code review for the Code Review Guide (whether it's alive or not) or SKF.

[ThunderSon]

@RiieCco @blabla1337 Hi SKF team :heart: Would you be interested to host code review recommendations?

@kingthorin should we just remove the sections that discuss code review?

[kingthorin]

I’m not against mentioning it, I just don’t think it’s the TG’s place to try to be authoritative in it.

[ThunderSon]

Something to keep in mind; sometimes checking the backend code is essential to validate the security (CRYP-04 is one example). Would that be considered stepping outside of the WSTG boundaries? Or would that be the actual test in that case? You can't actually see for example what encryption is being used and if it's securely implemented from outside.

[kingthorin]

Hmmm that's an interesting example, however, if you look through the Code Review section how/where do we draw the lines? There are some examples but they're java'centric. Maybe we don't care but the argument can easily be made by readers "You have java details why don't you have X details" where X is any language under the sun.

It also overlooks a ton of material like DB (at rest) encryption, drive encryption, S3 encryption, etc. and is more based on suggestions that testing.

To me while it's good for the TG to acknowledge and even discuss some techniques outside L7 remote testing, if we try to make the guide all encompassing it'll never hit all the right points.

I'll open a separate issue for CRYP-04 it's got it's own issues (ex: rough English, a personal author/editor reference, etc.)

[RiieCco]

@ThunderSon, @kingthorin, we would love to host this!! How can we help? :-)

[ThunderSon]

That's easy :yum: From your side, you'd need to have the code review for the test cases. They can be more wide, or not specific to the WSTG. From our side, we'll link which fits most to the tests. Pretty sure I can help with that as well :)

Let me know of your timelines and see this through.

[ThunderSon]

@kingthorin after @RiieCco was going through the first 2 chapters, the gray box sections seemed to need reformulation, and not full removal. Looking at this, I am worried that we could waste a lot of time on this, where as we can be restructuring a full test.

My suggestion comes now to close this issue and try to tackle things differently. This was too optimistic.

[kingthorin]

Alright that's fair. I'm okay with closing it if that's what makes sense now that it's been investigated.

@RiieCco if you have notes/thoughts on the content you did look at feel free to open other issues.

[github-actions[bot]]

Please comment if you are still working on this issue, as it has been inactive for 30 days. To give everyone a chance to contribute, we are releasing it to new contributors.

[patrickceg]

As a developer, my thoughts on code review or grey / white box:

• Anything where a human must manually look at the configuration or source code to find vulnerabilities should go in a different guide (secure coding guides, hardening guides, etc) because staring at source code is not really a "test"
  • I don't think there is a case where you can have a vulnerability found by staring at the code, but there's no way to write a test to exploit it. There are of course exploits that cannot be run reliably including exploitation of race conditions. However, if the probability of hitting a vulnerable piece of code is zero, it probably means that path is dead code (which now just goes in to software implementation guidelines).
• If there exists an automated static analysis tool to find the potentially dangerous code pattern that could be a vulnerability (example: Static analysis including SpotBugs to find null dereference in back-ends, OWASP Dependency Check to check library version numbers), then it is a test and should go in this guide
• As a separate topic, samples of vulnerable code or configurations can still go in this guide on a case-by-case basis, because that would be required if a system fails a test and the tester + developers must collaborate to find what the cause is / is the failure a false positive.

I agree with the other comments that enforcing some criteria like this could result in maybe tens of pages of content needing to be replaced by references to secure coding / configuration guides, or even being removed completely from wstg if the only way to find a vulnerability happens to be black box test or stare at code manually with nothing in between.

(I've just started back up on side projects so this is my first post to this Github in easily months)

I wasn't part of the original discussion, but I wouldn't mind closing this task just because we discovered it's too broad. I read we have multiple agreements that we would decrease the quality of the guide if we went in and blindly removed every "Grey Box Testing" with a code sample. We'd have to look at articles case-by-case, which means we need separate tasks.

As a note, if we decide to make tasks to define how we handle reviewing code, another article we need to look at is format strings https://github.com/OWASP/wstg/blob/master/document/4-Web_Application_Security_Testing/07-Input_Validation_Testing/13-Testing_for_Format_String_Injection.md where there's a manual code review section as well as some code samples.

[github-actions[bot]]

Please comment if you are still working on this issue, as it has been inactive for 30 days. To give everyone a chance to contribute, we are releasing it to new contributors.

[github-actions[bot]]

Please comment if you are still working on this issue, as it has been inactive for 30 days. To give everyone a chance to contribute, we are releasing it to new contributors.

[victoriadrake]

Closing for now. This idea needs further specificity and consideration of scope.