Skip to content

Conversation

camilaavilarinho
Copy link
Contributor

Adding the lab for Insecure Deserialization

Signed-off-by: Camila Vilarinho <[email protected]>
@camilaavilarinho camilaavilarinho changed the title [Draft] Add insecure deserialization lab Add insecure deserialization lab Sep 22, 2024

<!-- Full pattern of correct answer -->
<script id="correct0" type="plain/text">
const\s+data = JSON\.parse\(base64Decoded\)\; \s*
Copy link
Contributor

Choose a reason for hiding this comment

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

Overall correct0 looks good. There should be spaces around \. and ( and ) and \; because in JavaScript whitespace is optional around each.

const\s+data = JSON\.parse\(base64Decoded\)\; \s*
</script>
<script id="correct1" type="plain/text">
if \(data\.username && typeof\s+data\.username == ('string'|"string"|`string`) && data\.username\.length < 20\) \{ \s*
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, in correct1 I think there should be spaces around ( and \. and \) because whitespace is optional around them.

I think after typeof you should allow the following parameter to be surrounded by a parenthesized pair, that is, typeof ( \( data \. username \) | data \. username ) - treating typeof as a function/method call isn't insane, and JavaScript would accept it.

---
hints:
- present: "json"
text: the JSON built-in global object is witten in uppercase.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not everyone who takes the course will know JavaScript, and in particular, they may not be familiar with JavaScript syntax.

I suggest creating several hints for someone who's trying to fill in part1, but is getting stuck on one of the parts. Break down each part of the condition:

data.username && typeof data.username == 'string' && data.username.length < 20) {

So if they're missing "data.username", suggest that as a way to detect if it's there. Then, if they're missing the typeof, suggest that. Many won't know if the result is 'String' or 'string', help them there. And so on. Adding a few hint patterns can really help someone who understands the concept but can't quite get the syntax right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing it! I'll add your suggestions.

I tried to add more hints, but when I use "absent" for one of the parts, it still considers absent if the value is not in both parts. For example if I add "absent: length..." and then fill part 2 with length, the hint continues to show because length is not in part 1 as well. Am I doing something wrong?
Maybe is better if I change to have just one input area for both parts?

Copy link
Contributor

Choose a reason for hiding this comment

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

By default it only looks at the first part, which is index 0. If you want it to look at a different part, you need to give it an index: 1 or whatever the index is. You can see examples here:

If you want to check both places, currently you have to repeat the rule. If that happens a lot I guess we could add modify checker.js so you could provide index: LIST, but let's see if that happens often first...!

@david-a-wheeler
Copy link
Contributor

Hi - I'm really eager to get your lab "over the hump" and into the course. Will you be making these changes by Oct 9? Otherwise, I think I'll merge this in, and make a few fix up tweaks myself. I want to ensure people can use the wonderful material you've developed. It's so close, I want to get this completed and released.

Thank you again!

Signed-off-by: Camila Vilarinho <[email protected]>
@camilaavilarinho
Copy link
Contributor Author

Hi - I'm really eager to get your lab "over the hump" and into the course. Will you be making these changes by Oct 9? Otherwise, I think I'll merge this in, and make a few fix up tweaks myself. I want to ensure people can use the wonderful material you've developed. It's so close, I want to get this completed and released.

Thank you again!

Sorry the delay!

@david-a-wheeler
Copy link
Contributor

As noted earlier, I'm going to merge this draft lab in, and I'll make proposed tweaks as a separate PR to address the review comments above.

THANK YOU for all your work!!

@david-a-wheeler david-a-wheeler merged commit 956b581 into ossf:main Oct 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants