Skip to content

Fix path traversal vulnerability#233

Merged
lirantal merged 2 commits intoOWASP:masterfrom
rcowsill:fix/path-traversal
May 28, 2023
Merged

Fix path traversal vulnerability#233
lirantal merged 2 commits intoOWASP:masterfrom
rcowsill:fix/path-traversal

Conversation

@rcowsill
Copy link
Contributor

The first commit moves the tutorial routes out of routes/index.js and into a separate router. This is to avoid cluttering the top-level route setup with the tutorial routing code. The tutorial is effectively a microsite alongside the main application, so it seems reasonable to split it out like this.

The second commit replaces the /tutorial/:page route with individual routes for each page.

Currently the set of allowed pages is just listed explicitly. I considered generating the page list from the filesystem or using HenrikJoreteg/semi-static, but that would require moving the templates around to avoid serving the layout template.

Fixes #232

@lirantal lirantal self-requested a review March 6, 2023 10:19
rcowsill added 2 commits May 28, 2023 00:00
Add individual routes for each tutorial page instead of using a single
parameterized route.

The parameterized route allowed the user to pass unexpected :page
values. Relative paths could be specified to access arbitrary files
with extension ".html". Specifying any other extension would cause
express to dynamically require the module with the matching name. This
allowed loading of locally or globally installed modules, and also
permitted dependency enumeration.
@rcowsill rcowsill force-pushed the fix/path-traversal branch from 8f6803b to 69e3d0d Compare May 28, 2023 12:03
@lirantal
Copy link
Collaborator

Thanks!

Copy link
Collaborator

@lirantal lirantal left a comment

Choose a reason for hiding this comment

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

LGTM

@lirantal lirantal merged commit 5e71697 into OWASP:master May 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected path traversal vulnerability

3 participants