Skip to content

Commit

Permalink
[PLAY-1432] Playground Sanitization Security (#3624)
Browse files Browse the repository at this point in the history
**What does this PR do?** A clear and concise description with your
runway ticket url.

Runway https://runway.powerhrg.com/backlog_items/PLAY-1432

Anyone was able to input whatever erb into the playground. This was bad
because people could do shell scrips and other stuff by submitting
something like this `<%= system(“pwd”) %>`

The plan is to evaluate the ERB code coming in and if it has a method
that isn’t on the white list then the request is not processed

These methods have to be on the white list because the Parser parses
ruby and theses methods are needed to “run” erb

```
<<
To_s
+@
Freeze
```


More details about my thoughts here
https://huddle.powerapp.cloud/v2/projects/647/artifacts/6881
  • Loading branch information
markdoeswork authored Sep 12, 2024
1 parent 1724150 commit 360a1d8
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 8 deletions.
95 changes: 94 additions & 1 deletion playbook-website/app/controllers/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
require "will_paginate"
require "playbook/pagination_renderer"
require "will_paginate/array"
require "parser/current"
require "erb"
require "ostruct"

class PagesController < ApplicationController
Expand Down Expand Up @@ -145,8 +147,99 @@ def kit_playground_rails
render "pages/rails_in_react_playground", layout: "layouts/fullscreen"
end

def parse_erb(code)
parsed = code.scan(/<%=?\s*(.*?)\s*%>/).flatten.join("\n")
Parser::CurrentRuby.parse(parsed)
end

def detect_ruby_methods(root_node)
stack = [root_node] # Initialize the stack with the root node
methods = []
until stack.empty?
node = stack.pop # Get the current node from the stack

next unless node.is_a?(Parser::AST::Node)

# Check if the current node is a `:send` node and has `:system` as the method name
methods.push(node.children[1]) if node.type == :send
# Push all child nodes onto the stack
stack.concat(node.children)
end
methods
end

def white_list
%i[<< to_s pb_rails +@ freeze]
end

# Define methods outside of the main method
def add_xstr_node(node, xstr_nodes)
xstr_nodes << node if node.type == :xstr
end

def add_gvar_node(node, gvar_nodes)
gvar_nodes << node if node.type == :gvar
end

def traverse_node_gvar(node, xstr_nodes)
case node
when Parser::AST::Node
add_gvar_node(node, xstr_nodes)
# Recur for each child node
node.children.each do |child|
traverse_node_gvar(child, xstr_nodes)
end
end
end

def detect_gvar_nodes(ast_node)
gvar_nodes = []
# Start traversing from the root node
traverse_node_gvar(ast_node, gvar_nodes)
gvar_nodes
end

def traverse_node(node, xstr_nodes)
case node
when Parser::AST::Node
add_xstr_node(node, xstr_nodes)
# Recur for each child node
node.children.each do |child|
traverse_node(child, xstr_nodes)
end
end
end

def detect_xstr_nodes(ast_node)
xstr_nodes = []
# Start traversing from the root node
traverse_node(ast_node, xstr_nodes)
xstr_nodes
end

def is_erb_safe?(code)
node = parse_erb(code)
detect_gvar_nodes(parse_erb(erb_code_params)).empty? &&
detect_ruby_methods(node) - white_list == [] &&
detect_xstr_nodes(parse_erb(erb_code_params)).empty?
end

def unsafe_code
detect_ruby_methods(parse_erb(erb_code_params)).uniq - white_list
end

def playground_response
if is_erb_safe?(erb_code_params)
erb_code_params
elsif unsafe_code.any?
"The code provided can't be run please remove these methods: #{unsafe_code.join(', ')}"
else
"The code provided can't be run. Only use pb_rails."
end
end

def rails_pg_render
render inline: erb_code_params
render inline: playground_response
rescue => e
render json: { error: e }, status: 400
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ export const SideBarNavItems = [
leftIcon:"vial"
},

// {
// name: "Playground",
// key: "top-nav-item-6",
// link: "/kit_playground_rails",
// children: false,
// leftIcon:"laptop-code"
// }
{
name: "Playground",
key: "top-nav-item-6",
link: "/kit_playground_rails",
children: false,
leftIcon:"laptop-code"
}
]
2 changes: 2 additions & 0 deletions playbook-website/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
get "kits/:name/sandpack", to: "pages#kit_show_new", as: "kit_show_new"
get "kits/:name/rails_in_react", to: "pages#rails_in_react", as: "rails_in_react"
get "kits/:name/rails_raw", to: "pages#rails_raw", as: "rails_raw"
get "kit_playground_rails", to: "pages#kit_playground_rails", as: "kit_playground_rails"
post "rails_pg_render", to: "pages#rails_pg_render", as: "rails_pg_render"

# Docs
get "guides/:parent", to: "guides#md_doc", as: "guides_parent"
Expand Down

0 comments on commit 360a1d8

Please sign in to comment.