Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 151 additions & 0 deletions docs/labs/insecure-deserialization.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1">
<link rel="stylesheet" href="https://best.openssf.org/assets/css/style.css">
<link rel="stylesheet" href="checker.css">
<script src="js-yaml.min.js"></script>
<script src="checker.js"></script>
<link rel="license" href="https://creativecommons.org/licenses/by/4.0/">

<!-- See create_labs.md for how to create your own lab! -->

<!-- Sample expected answer -->
<script id="expected0" type="plain/text">
const data = JSON.parse(base64Decoded);
</script>
<!--
-->
<script id="expected1" type="plain/text">
if (data.username && typeof data.username == 'string' && data.username.length < 20) {
</script>

<!-- Full pattern of correct answer -->
<script id="correct0" type="plain/text">
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*
</script>

<script id="info" type="application/yaml">
---
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...!

- absent: data.username
index: 1
text: Check if the data object has a property called username. You can do this by referencing data.username.
- absent: \&\&
index: 1
text: To combine multiple conditions in JavaScript use &&. This operator means 'and', so both conditions must be true for the entire statement to pass.
- absent: typeof
index: 1
text: Use typeof to check the type of the operand's value.
- present: typeof data.username == 'String'
index: 1
text: When using typeof, JavaScript expects "string" all lowercase.
- absent: length
index: 1
text: check if the length of the string is smaller than 20 characters.

# debug: true
</script>
</head>
<body>
<!-- For GitHub Pages formatting: -->
<div class="container-lg px-3 my-5 markdown-body">
<h1>Lab Exercise Insecure Deserialization</h1>
<p>
This is a lab exercise on developing secure software.
For more information, see the <a href="introduction.html" target="_blank">introduction to
the labs</a>.

<p>
<h2>Task</h2>
<p>
<b>Please change the code below to prevent insecure deserialization vulnerability.</b>

<p>
<h2>Background</h2>
<p>
Insecure Deserialization happens when the application’s deserialization process is exploited, allowing an attacker to manipulate the serialized data and pass harmful data into the application code.
<p>
The safest way to prevent this vulnerability is to not accept serialized objects from untrusted sources (user-controllable data). However, if you must accept them, there are some mitigations you can implement. In this lab, we will apply a couple of them.

<p>
<h2>Task Information</h2>
<p>

<p>
The code below is called after an application login page. After login, a cookie is set up with the user profile, then in the homepage the cookie is deserialized and uses the username in a greeting message.
<p>
If you take a closer look at this code, you’ll see that it’s using <tt>eval()</tt> to deserialize the data from the cookie. This can be very dangerous as <tt>eval()</tt> evaluates a string as JavaScript code, which means any code inside that string will be executed, opening the possibility of Remote Code Execution (RCE) and Code Injection attacks.
<p>
For this lab we want to fix this by using an approach that prevents code execution, we will also add input validation to make sure the data we receive is what we are expecting and nothing more than that.
<ol>
<li>
Replace <tt>eval()</tt> with <tt>JSON.parse()</tt>. JSON.parse( ) does not execute any JavaScript code like functions or methods, making it safer than some other serialization libraries.
</li>
<li>Besides checking if <tt>data.username</tt> exists, also check that the username is a string and not bigger than 20 characters.</li>
</ol>


<p>
Use the “hint” and “give up” buttons if necessary.

<p>
<h2>Interactive Lab (<span id="grade"></span>)</h2>
<p>
<p>
Change the code below, adding the mitigation steps to prevent Insecure Deserialization:
<ol>
<li>Use a deserialization approach that prevents code execution.</li>
<li>Validate the username making sure it is used only if it's a <b>string</b> and no longer than <b>20 characters</b>.</li>
</ol>
<form id="lab">

<pre><code>
const express = require('express');
const cookieParser = require('cookie-parser');

const app = express();

app.use(express.json());
app.use(cookieParser());

app.get('/', (req, res) => {
if (req.cookies.profile) {
try {
const base64Decoded = Buffer.from(req.cookies.profile, 'base64').toString('utf8');
<input id="attempt0" type="text" size="60" spellcheck="false" value="const data = eval('(' + base64Decoded + ')');">

<input id="attempt1" type="text" size="60" spellcheck="false" value="if (data.username) {">
res.send(`Hello ${data.username}`);
}

} catch (err) {
res.send('An error occurred: ' + err.message);
}
} else {
res.send("Please Login");
}
});
</code></pre>


<button type="button" class="hintButton">Hint</button>
<button type="button" class="resetButton">Reset</button>
<button type="button" class="giveUpButton">Give up</button>
<br><br>
<p>
<i>This lab was developed by Camila Vilarinho.</i>
<br><br>
<p id="correctStamp" class="small">
<textarea id="debugData" class="displayNone" rows="20" cols="65" readonly>
</textarea>
</form>
</div><!-- End GitHub pages formatting -->
</body>
</html>
Loading