1
0
mirror of https://github.com/laurent22/joplin.git synced 2024-11-24 08:12:24 +02:00

Doc: Added info about data escaping in coding style

This commit is contained in:
Laurent Cozic 2023-03-23 16:10:52 +00:00
parent 104e752634
commit 5995dc81f3

View File

@ -190,6 +190,123 @@ const foo = () => {
As much as possible, avoid default parameters in **function definitions** and optional fields in **interface definitions**. When all parameters are required, it is much easier to refactor the code because the compiler will automatically catch any missing parameters.
## Escape variables
[XSS](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html) is one of the most common vulnerabilities in today's code. These vulnerabilities are often difficult to spot because they are not errors, they often won't fail any test units and the program will work just fine with 99% of input. Yet that remaining 1% can be exploited and used to steal user information, crash the app, etc.
If you search for ["XSS" in the Joplin git log](https://github.com/laurent22/joplin/search?q=xss&type=commits) you'll find several security vulnerabilities that have been fixed over the year, and that happened in various places that are hard to predict. So we need to be careful with this and make sure we correctly escape user content.
How you escape the data depends on where you are going to insert it so there's no single function that's going to cover all cases.
### To insert into a JS script
Use `JSON.stringify()`. For example:
```ts
const jsCode = `const data = ${JSON.stringify(dynamicallyGeneratedData)};`
```
### To insert into an HTML string
You need to convert special characters to HTML entities, which we usually do using the `html-entities` package. For example:
```ts
// We generally use this rather verbose pattern, but there
// are also helper functions that you may be able to use
// depending on the package.
const Entities = require('html-entities').AllHtmlEntities;
const htmlentities = new Entities().encode;
const html = `<a href="${htmlentities(attributes)}">${htmlentities(content)}</a>`;
```
### To insert into a URL
It depends on what you're trying to do. To insert a query parameter, use `encodeURIComponent`
```ts
const url = `https://example.com/?page=${encodeURIComponent(page)}`;
```
If you want to encode a full URL, use `encodeURI`:
```ts
encodeURI('https://domain.com/path to a document.pdf');
// 'https://domain.com/path%20to%20a%20document.pdf'
```
### Escape as late as possible
Ideally the application should only deal with raw, unencoded data, so it means data should be decoded and encoded at the application boundaries. Doing so means we avoid accidentally double-escaping data, or having to encode/decode within the app, which is error prone.
In practice it means as soon as we get user input, we should decode it to the application-specific format (for example by calling `JSON.parse` on the input). And likewise we should only escape the data when it needs to be printed or exported.
**BAD**
```ts
let parameters = `id=${encodeURIComponent(id)}&time=${encodeURIComponent(Date.now())}`;
// Clumsy string concatenation because we're dealing with already escaped data.
// and we have to remember to encode every time:
parameters += `&other=${encodeURIComponent(otherParam)}`;
const url = `https://example.com?${parameters}`
```
**GOOD**
```ts
// Keep the data as an object
const parameters = {
id: id,
timestamp: Date.now(),
};
// Then we can easily add to it without string concatenation:
parameters.other = otherParam;
// We escape only when it is needed:
const url = `https://example.com?${new URLSearchParams(parameters).toString()}`
```
### Make wrong code look wrong
To name variables that are already escaped we used the technique described in "[Make wrong code look wrong](https://www.joelonsoftware.com/2005/05/11/making-wrong-code-look-wrong/)". We add a suffix to indicate the content of the variable and to make it clear it has already been escaped. It means that the code will look wrong if a variable is inserted in a string and it does not have a suffix. For example:
**BAD:**
```ts
const userContent = queryParameters.page;
// ...
// later:
// ...
const html = `<div>${userContent}</div>`
// The above code looks wrong because it appears we're
// inserting user input as is in the document, and
// indeed we are. Wrong code looks wrong.
```
**GOOD:**
```ts
// Here we escape the data immediately - and we add an
// "html" prefix to specify that we have escaped the data
// and that the variable content is actual HTML.
const userContentHtml = htmlentities(queryParameters.page);
// ...
// later:
// ...
const html = `<div>${userContentHtml}</div>`
// This is correct and because we've added the "html" suffix
// we know that this variable can be safely added to an HTML
// string.
```
# React
## Use function components for new code