Skip to content

Commit

Permalink
security: multiple reported CVE fixes (#1515)
Browse files Browse the repository at this point in the history
* update out of date license

* update typing / refactor

* fix arbitrarty path injection

* use markdown sanatizer to prevent XSS CWE-79

* fix CWE-918 SSRF by validating url and mime type

* add security docs

* update recipe-scrapers

* resolve DOS from arbitrary url

* update changelog

* bump version

* add ref to #1506

* add #1511 to changelog

* use requests decoder

* actually fix encoding issue
  • Loading branch information
hay-kot authored Jul 31, 2022
1 parent 483f789 commit 13850cd
Show file tree
Hide file tree
Showing 23 changed files with 401 additions and 118 deletions.
30 changes: 0 additions & 30 deletions docs/docs/changelog/.template.md

This file was deleted.

126 changes: 126 additions & 0 deletions docs/docs/changelog/v1.0.0beta-4.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
### Security

#### v1.0.0beta-3 and Under - Recipe Scraper: Server Side Request Forgery Lead To Denial Of Service

!!! error "CWE-918: Server-Side Request Forgery (SSRF)"
In this case if a attacker try to load a huge file then server will try to load the file and eventually server use its all memory which will dos the server

##### Mitigation

HTML is now scraped via a Stream and canceled after a 15 second timeout to prevent arbitrary data from being loaded into the server.

#### v1.0.0beta-3 and Under - Recipe Assets: Remote Code Execution

!!! error "CWE-1336: Improper Neutralization of Special Elements Used in a Template Engine"
As a low privileged user, Create a new recipe and click on the "+" to add a New Asset.
Select a file, then proxy the request that will create the asset.

Since mealie/routes/recipe/recipe_crud_routes.py:306 is calling slugify on the name POST parameter, we use $ which slugify() will remove completely.

Since mealie/routes/recipe/recipe_crud_routes.py:306 is concatenating raw user input from the extension POST parameter into the variable file_name, which ultimately gets used when writing to disk, we can use a directory traversal attack in the extension (e.g. ./../../../tmp/pwn.txt) to write the file to arbitrary location on the server.

As an attacker, now that we have a strong attack primitive, we can start getting creative to get RCE. Since the files were being created by root, we could add an entry to /etc/passwd, create a crontab, etc. but since there was templating functionality in the application that peaked my interest. The PoC in the HTTP request above creates a Jinja2 template at /app/data/template/pwn.html. Since Jinja2 templates execute Python code when rendered, all we have to do now to get code execution is render the malicious template. This was easy enough.

##### Mitigation

We've added proper path sanitization to ensure that the user is not allowed to write to arbitrary locations on the server.

!!! warning "Breaking Change Incoming"
As this has shown a significant area of exposure in the templates that Mealie was provided for exporting recipes, we'll be removing this feature in the next Beta release and will instead rely on the community to provide tooling around transforming recipes using templates. This will significantly limit the possible exposure of users injecting malicious templates into the application. The template functionality will be completely removed in the next beta release v1.0.0beta-5

#### All version Markdown Editor: Cross Site Scripting

!!! error "CWE-79: Cross-site Scripting (XSS) - Stored"
A low privilege user can insert malicious JavaScript code into the Recipe Instructions which will execute in another person's browser that visits the recipe.

`<img src=x onerror=alert(document.domain)>`

##### Mitigation

This issues is present on all pages that allow markdown input. This error has been mitigated by wrapping the 3rd Party Markdown component and using the `domPurify` library to strip out the dangerous HTML.

#### v1.0.0beta-3 and Under - Image Scraper: Server-Side Request Forgery

!!! error "CWE-918: Server-Side Request Forgery (SSRF)"
In the recipe edit page, is possible to upload an image directly or via an URL provided by the user. The function that handles the fetching and saving of the image via the URL doesn't have any URL verification, which allows to fetch internal services.

Furthermore, after the resource is fetch, there is no MIME type validation, which would ensure that the resource is indeed an image. After this, because there is no extension in the provided URL, the application will fallback to jpg, and original for the image name.

Then the result is saved to disk with the original.jpg name, that can be retrieved from the following URL: http://<domain>/api/media/recipes/<recipe-uid>/images/original.jpg. This file will contain the full response of the provided URL.

**Impact**

An attacker can get sensitive information of any internal-only services running. For example, if the application is hosted on Amazon Web Services (AWS) platform, its possible to fetch the AWS API endpoint, https://169.254.169.254, which returns API keys and other sensitive metadata.

##### Mitigation

Two actions were taken to reduce exposure to SSRF in this case.

1. The application will not prevent requests being made to local resources by checking for localhost or 127.0.0.1 domain names.
2. The mime-type of the response is now checked prior to writing to disk.

If either of the above actions prevent the user from uploading images, the application will alert the user of what error occurred.

### Bug Fixes

- For erroneously-translated datetime config ([#1362](https://github.com/hay-kot/mealie/issues/1362))
- Fixed text color on RecipeCard in RecipePrintView and implemented ingredient sections ([#1351](https://github.com/hay-kot/mealie/issues/1351))
- Ingredient sections lost after parsing ([#1368](https://github.com/hay-kot/mealie/issues/1368))
- Increased float rounding precision for CRF parser ([#1369](https://github.com/hay-kot/mealie/issues/1369))
- Infinite scroll bug on all recipes page ([#1393](https://github.com/hay-kot/mealie/issues/1393))
- Fast fail of bulk importer ([#1394](https://github.com/hay-kot/mealie/issues/1394))
- Bump @mdi/js from 5.9.55 to 6.7.96 in /frontend ([#1279](https://github.com/hay-kot/mealie/issues/1279))
- Bump @nuxtjs/i18n from 7.0.3 to 7.2.2 in /frontend ([#1288](https://github.com/hay-kot/mealie/issues/1288))
- Bump date-fns from 2.23.0 to 2.28.0 in /frontend ([#1293](https://github.com/hay-kot/mealie/issues/1293))
- Bump fuse.js from 6.5.3 to 6.6.2 in /frontend ([#1325](https://github.com/hay-kot/mealie/issues/1325))
- Bump core-js from 3.17.2 to 3.23.1 in /frontend ([#1383](https://github.com/hay-kot/mealie/issues/1383))
- All-recipes page now sorts alphabetically ([#1405](https://github.com/hay-kot/mealie/issues/1405))
- Sort recent recipes by created_at instead of date_added ([#1417](https://github.com/hay-kot/mealie/issues/1417))
- Only show scaler when ingredients amounts enabled ([#1426](https://github.com/hay-kot/mealie/issues/1426))
- Add missing types for API token deletion ([#1428](https://github.com/hay-kot/mealie/issues/1428))
- Entry nutrition checker ([#1448](https://github.com/hay-kot/mealie/issues/1448))
- Use == operator instead of is_ for sql queries ([#1453](https://github.com/hay-kot/mealie/issues/1453))
- Use `mtime` instead of `ctime` for backup dates ([#1461](https://github.com/hay-kot/mealie/issues/1461))
- Mealplan pagination ([#1464](https://github.com/hay-kot/mealie/issues/1464))
- Properly use pagination for group event notifies ([#1512](https://github.com/hay-kot/mealie/pull/1512))

### Documentation

- Add go bulk import example ([#1388](https://github.com/hay-kot/mealie/issues/1388))
- Fix old link
- Pagination and filtering, and fixed a few broken links ([#1488](https://github.com/hay-kot/mealie/issues/1488))

### Features

- Toggle display of ingredient references in recipe instructions ([#1268](https://github.com/hay-kot/mealie/issues/1268))
- Add custom scaling option ([#1345](https://github.com/hay-kot/mealie/issues/1345))
- Implemented "order by" API parameters for recipe, food, and unit queries ([#1356](https://github.com/hay-kot/mealie/issues/1356))
- Implement user favorites page ([#1376](https://github.com/hay-kot/mealie/issues/1376))
- Extend Apprise JSON notification functionality with programmatic data ([#1355](https://github.com/hay-kot/mealie/issues/1355))
- Mealplan-webhooks ([#1403](https://github.com/hay-kot/mealie/issues/1403))
- Added "last-modified" header to supported record types ([#1379](https://github.com/hay-kot/mealie/issues/1379))
- Re-write get all routes to use pagination ([#1424](https://github.com/hay-kot/mealie/issues/1424))
- Advanced filtering API ([#1468](https://github.com/hay-kot/mealie/issues/1468))
- Restore frontend sorting for all recipes ([#1497](https://github.com/hay-kot/mealie/issues/1497))
- Implemented local storage for sorting and dynamic sort icons on the new recipe sort card ([1506](https://github.com/hay-kot/mealie/pull/1506))
- create new foods and units from their Data Management pages ([#1511](https://github.com/hay-kot/mealie/pull/1511))

### Miscellaneous Tasks

- Bump dev deps ([#1418](https://github.com/hay-kot/mealie/issues/1418))
- Bump @vue/runtime-dom in /frontend ([#1423](https://github.com/hay-kot/mealie/issues/1423))
- Backend page_all route cleanup ([#1483](https://github.com/hay-kot/mealie/issues/1483))

### Refactor

- Remove depreciated repo call ([#1370](https://github.com/hay-kot/mealie/issues/1370))

### Hotfix

- Tame typescript beast

### UI

- Improve parser ui text display ([#1437](https://github.com/hay-kot/mealie/issues/1437))

<!-- generated by git-cliff -->
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
version: "3.7"
services:
mealie-frontend:
image: hkotel/mealie:frontend-v1.0.0beta-3
image: hkotel/mealie:frontend-v1.0.0beta-4
container_name: mealie-frontend
depends_on:
- mealie-api
Expand All @@ -23,7 +23,7 @@ services:
volumes:
- mealie-data:/app/data/ # (3)
mealie-api:
image: hkotel/mealie:api-v1.0.0beta-3
image: hkotel/mealie:api-v1.0.0beta-4
container_name: mealie-api
depends_on:
- postgres
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ SQLite is a popular, open source, self-contained, zero-configuration database th
version: "3.7"
services:
mealie-frontend:
image: hkotel/mealie:frontend-v1.0.0beta-3
image: hkotel/mealie:frontend-v1.0.0beta-4
container_name: mealie-frontend
environment:
# Set Frontend ENV Variables Here
Expand All @@ -23,7 +23,7 @@ services:
volumes:
- mealie-data:/app/data/ # (3)
mealie-api:
image: hkotel/mealie:api-v1.0.0beta-3
image: hkotel/mealie:api-v1.0.0beta-4
container_name: mealie-api
volumes:
- mealie-data:/app/data/
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/overrides/api.html

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions docs/mkdocs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ nav:
- Improving Ingredient Parser: "contributors/guides/ingredient-parser.md"

- Change Log:
- v1.0.0beta-4: "changelog/v1.0.0beta-4.md"
- v1.0.0beta-3: "changelog/v1.0.0beta-3.md"
- v1.0.0beta-2: "changelog/v1.0.0beta-2.md"
- v1.0.0 Beta: "changelog/v1.0.0.md"
Expand Down
7 changes: 2 additions & 5 deletions frontend/components/Domain/Recipe/RecipeIngredients.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<v-list-item dense @click="toggleChecked(index)">
<v-checkbox hide-details :value="checked[index]" class="pt-0 my-auto py-auto" color="secondary" />
<v-list-item-content :key="ingredient.quantity">
<VueMarkdown class="ma-0 pa-0 text-subtitle-1 dense-markdown" :source="ingredientDisplay[index]" />
<SafeMarkdown class="ma-0 pa-0 text-subtitle-1 dense-markdown" :source="ingredientDisplay[index]" />
</v-list-item-content>
</v-list-item>
</div>
Expand All @@ -22,14 +22,11 @@
<script lang="ts">
import { computed, defineComponent, reactive, toRefs } from "@nuxtjs/composition-api";
// @ts-ignore vue-markdown has no types
import VueMarkdown from "@adapttive/vue-markdown";
import { parseIngredientText } from "~/composables/recipes";
import { RecipeIngredient } from "~/types/api-types/recipe";
export default defineComponent({
components: {
VueMarkdown,
},
components: {},
props: {
value: {
type: Array as () => RecipeIngredient[],
Expand Down
5 changes: 1 addition & 4 deletions frontend/components/Domain/Recipe/RecipeInstructions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@
<v-expand-transition>
<div v-show="!isChecked(index) && !edit" class="m-0 p-0">
<v-card-text class="markdown">
<VueMarkdown class="markdown" :source="step.text"> </VueMarkdown>
<SafeMarkdown class="markdown" :source="step.text" />
<div v-if="cookMode && step.ingredientReferences && step.ingredientReferences.length > 0">
<v-divider class="mb-2"></v-divider>
<div
Expand All @@ -219,8 +219,6 @@

<script lang="ts">
import draggable from "vuedraggable";
// @ts-ignore vue-markdown has no types
import VueMarkdown from "@adapttive/vue-markdown";
import {
ref,
toRefs,
Expand All @@ -245,7 +243,6 @@ interface MergerHistory {
export default defineComponent({
components: {
VueMarkdown,
draggable,
},
props: {
Expand Down
7 changes: 1 addition & 6 deletions frontend/components/Domain/Recipe/RecipeNotes.vue
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
{{ note.title }}
</v-card-title>
<v-card-text>
<VueMarkdown :source="note.text"> </VueMarkdown>
<SafeMarkdown :source="note.text" />
</v-card-text>
</div>
</div>
Expand All @@ -30,15 +30,10 @@
</template>

<script lang="ts">
// @ts-ignore vue-markdown has no types
import VueMarkdown from "@adapttive/vue-markdown";
import { defineComponent } from "@nuxtjs/composition-api";
import { RecipeNote } from "~/types/api-types/recipe";
export default defineComponent({
components: {
VueMarkdown,
},
props: {
value: {
type: Array as () => RecipeNote[],
Expand Down
9 changes: 3 additions & 6 deletions frontend/components/Domain/Recipe/RecipePrintView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</section>

<v-card-text class="px-0">
<VueMarkdown :source="recipe.description" />
<SafeMarkdown :source="recipe.description" />
</v-card-text>

<!-- Ingredients -->
Expand Down Expand Up @@ -47,7 +47,7 @@
{{ step.title }}
</h4>
<h5>{{ $t("recipe.step-index", { step: stepIndex + instructionSection.stepOffset + 1 }) }}</h5>
<VueMarkdown :source="step.text" class="recipe-step-body" />
<SafeMarkdown :source="step.text" class="recipe-step-body" />
</div>
</div>
</div>
Expand All @@ -60,7 +60,7 @@
<div v-for="(note, index) in recipe.notes" :key="index + 'note'">
<div class="print-section">
<h4>{{ note.title }}</h4>
<VueMarkdown :source="note.text" class="note-body" />
<SafeMarkdown :source="note.text" class="note-body" />
</div>
</div>
</section>
Expand All @@ -69,8 +69,6 @@

<script lang="ts">
import { defineComponent, computed } from "@nuxtjs/composition-api";
// @ts-ignore vue-markdown has no types
import VueMarkdown from "@adapttive/vue-markdown";
import RecipeTimeCard from "~/components/Domain/Recipe/RecipeTimeCard.vue";
import { Recipe, RecipeIngredient, RecipeStep } from "~/types/api-types/recipe";
import { parseIngredientText } from "~/composables/recipes";
Expand All @@ -89,7 +87,6 @@ type InstructionSection = {
export default defineComponent({
components: {
RecipeTimeCard,
VueMarkdown,
},
props: {
recipe: {
Expand Down
8 changes: 1 addition & 7 deletions frontend/components/global/MarkdownEditor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,15 @@
dense
rows="4"
/>
<VueMarkdown v-else :source="value" />
<SafeMarkdown v-else :source="value" />
</div>
</template>

<script lang="ts">
// @ts-ignore vue-markdown has no types
import VueMarkdown from "@adapttive/vue-markdown";
import { defineComponent, computed, ref } from "@nuxtjs/composition-api";
export default defineComponent({
name: "MarkdownEditor",
components: {
VueMarkdown,
},
props: {
value: {
type: String,
Expand Down
42 changes: 42 additions & 0 deletions frontend/components/global/SafeMarkdown.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<template>
<VueMarkdown :source="sanitizeMarkdown(source)"></VueMarkdown>
</template>

<script lang="ts">
// @ts-ignore vue-markdown has no types
import VueMarkdown from "@adapttive/vue-markdown";
import { defineComponent } from "@nuxtjs/composition-api";
import DOMPurify from "isomorphic-dompurify";
export default defineComponent({
components: {
VueMarkdown,
},
props: {
source: {
type: String,
default: "",
},
},
setup() {
function sanitizeMarkdown(rawHtml: string | null | undefined): string {
if (!rawHtml) {
return "";
}
const sanitized = DOMPurify.sanitize(rawHtml, {
USE_PROFILES: { html: true },
// TODO: some more thought could be put into what is allowed and what isn't
ALLOWED_TAGS: ["img", "div", "p"],
ADD_ATTR: ["src", "alt", "height", "width", "class"],
});
return sanitized;
}
return {
sanitizeMarkdown,
};
},
});
</script>
Loading

0 comments on commit 13850cd

Please sign in to comment.