Modify rule S5146 - Support Location header (#315)

rules/S5146/csharp/rule.adoc

@@ -5,21 +5,18 @@ include::../description.adoc[]
 ----
 using Microsoft.AspNetCore.Mvc;
 
-namespace WebApplicationDotNetCore.Controllers
+public class HomeController : Controller
 {
-    public class RSPEC5146OpenRedirectNoncompliantController : Controller
+
+    public IActionResult RedirectMe(string url)
     {
-        public IActionResult Index()
+        return Redirect(url);
-        {
+    }
-            return View();
-        }
 
-        private readonly string[] whiteList = { "https://www.sonarsource.com" };
+    public IActionResult SetLocationHeader(string url)
-
+    {
-        public IActionResult RedirectMe(string url)
+        Response.Headers["Location"] = url; // Noncompliant
-        {
+        return StatusCode(302);
-            return Redirect(url);
-        }
     }
 }
 ----
@@ -27,30 +24,21 @@ namespace WebApplicationDotNetCore.Controllers
 == Compliant Solution
 
 ----
-using System.Linq;
 using Microsoft.AspNetCore.Mvc;
 
-namespace WebApplicationDotNetCore.Controllers
+public class HomeController : Controller
 {
-    public class RSPEC5146OpenRedirectCompliantController : Controller
+    private readonly string[] whiteList = { "/", "/login", "/logout" };
+
+    public IActionResult RedirectMe(string url)
     {
-        public IActionResult Index()
+        // Match the incoming URL against a whitelist
+        if (!whiteList.Contains(url))
         {
-            return View();
+            return BadRequest();
         }
 
-        private readonly string[] whiteList = { "https://www.sonarsource.com" };
+        return Redirect(url);
-
-        public IActionResult RedirectMe(string url)
-        {
-            // Match the incoming URL against a whitelist
-            if (!whiteList.Contains(url))
-            {
-                return BadRequest();
-            }
-
-            return Redirect(url);
-        }
     }
 }
 ----

rules/S5146/java/rule.adoc

@@ -8,6 +8,13 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO
   resp.sendRedirect(location); // Noncompliant
 }
 ----
+----
+protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+  String location = req.getParameter("url");
+  resp.setStatus(302);
+  resp.setHeader("Location", location); // Noncompliant
+}
+----
 
 == Compliant Solution
 

rules/S5146/javascript/rule.adoc

@@ -3,11 +3,18 @@ include::../description.adoc[]
 == Noncompliant Code Example
 
 ----
-function (req, res) {
+function redirect(req, res) {
   const url = req.query.url; // user controlled input
 
   res.redirect(url); // Noncompliant
 }
+
+function setLocationHeader(req, res) {
+  const url = req.query.url; // user controlled input
+
+  res.location(url); // Noncompliant
+  res.sendStatus(302);
+}
 ----
 
 == Compliant Solution
@@ -23,7 +30,7 @@ function isValidUrl(url) {
   return false;
 }
 
-function (req, res) {
+function redirect(req, res) {
   const url = req.query.url; // user controlled input
 
   if(isValidUrl(url)) {

rules/S5146/php/rule.adoc

@@ -8,22 +8,70 @@ This problem could be mitigated in any of the following ways:
 
 == Noncompliant Code Example
 
+Symfony
+
 ----
-$url = $this->request->getQuery("url");
+public function redirect(Request $request)
-return $this->redirect($url); // Noncompliant
+{
+  $url = $request->query->get('url');
+  return $this->redirect($url); // Noncompliant
+}
+
+public function setLocatioHeader(Request $request)
+{
+  $url = $request->query->get('url');
+  $response = new Response('Redirecting...', 302);
+  $response->headers->set('Location', $url); // Noncompliant
+  return $response;
+}
+----
+
+Laravel
+----
+public function redirect(Request $request)
+{
+  $url = $request->input('url');
+  return $this->redirect($url); // Noncompliant
+}
+
+public function setLocatioHeader(Request $request)
+{
+  $url = $request->input('url');
+  return response("", 302)
+    ->header('Location', $url) // Noncompliant
+}
 ----
 
 == Compliant Solution
 
+Symfony
+
 ----
-$whitelist = array(
+public function redirect(Request $request)
-  "https://www.sonarsource.com/"
+{
-);
+  $url = $request->query->get('url');
-$url = $this->request->getQuery("url");
+  $allowedUrls = ["/index", "/login", "/logout"];
-if (in_array($url, $whitelist)) {
+
-  return $this->redirect($url);
+  if (in_array($url, $allowedUrls, true)) {
-} else {
+    return $this->redirect($url);
-  throw new ForbiddenException();
+  } else {
+    $this->redirect("/");
+  }
+}
+----
+
+Laravel
+----
+public function redirect(Request $request)
+{
+  $url = $request->input('url');
+  $allowedUrls = ["/index", "/login", "/logout"];
+
+  if (in_array($url, $allowedUrls, true)) {
+    return $this->redirect($url);
+  } else {
+    return redirect("/");
+  }
 }
 ----
 

rules/S5146/python/rule.adoc

@@ -5,12 +5,19 @@ include::../description.adoc[]
 Flask
 
 ----
-from flask import request, redirect
+from flask import request, redirect, Response
 
-@app.route('move')
+@app.route('flask_redirect')
-def move():
+def flask_redirect():
     url = request.args["next"]
-    return redirect(url) # Noncompliant
+    return redirect(url)  # Noncompliant
+
+@app.route('set_location_header')
+def set_location_header():
+    url = request.args["next"]
+    response = Response("redirecting...", 302)
+    response.headers['Location'] = url  # Noncompliant
+    return response
 ----
 
 Django
@@ -18,9 +25,15 @@ Django
 ----
 from django.http import HttpResponseRedirect
 
-def move(request):
+def http_responser_redirect(request):
     url = request.GET.get("next", "/")
-    return HttpResponseRedirect(url) # Noncompliant
+    return HttpResponseRedirect(url)  # Noncompliant
+
+def set_location_header(request):
+    url = request.GET.get("next", "/")
+    response = HttpResponse(status=302)
+    response['Location'] = url  # Noncompliant
+    return response
 ----
 
 == Compliant Solution
@@ -28,12 +41,12 @@ def move(request):
 Flask
 
 ----
-from flask import request, redirect, url_for
+from flask import request, redirect, Response, url_for
 
-@app.route('move')
+@app.route('flask_redirect')
-def move():
+def flask_redirect():
     endpoint = request.args["next"]
-    return redirect(url_for(endpoint)) # Compliant
+    return redirect(url_for(endpoint))  # Compliant
 ----
 
 Django
@@ -44,11 +57,11 @@ from urllib.parse import urlparse
 
 DOMAINS_WHITELIST = ['www.example.com', 'example.com']
 
-def move(request):
+def http_responser_redirect(request):
     url = request.GET.get("next", "/")
     parsed_uri = urlparse(url)
     if parsed_uri.netloc in DOMAINS_WHITELIST:
-        return HttpResponseRedirect(url) # Compliant
+        return HttpResponseRedirect(url)  # Compliant
     return HttpResponseRedirect("/")
 ----