SONARPY-3640: Update S8392 to unify with Flask applications (#787)

Co-authored-by: Thomas Serre <thomas.serre@sonarsource.com>
Co-authored-by: Thomas Serre <118730793+thomas-serre-sonarsource@users.noreply.github.com>
GitOrigin-RevId: 66f75d4a18b185ac555442d8a1a422e3b6eb770c

Author: 2 people

…tAPIBindToAllNetworkInterfacesCheck.java …cks/BindToAllNetworkInterfacesCheck.javapython-checks/src/main/java/org/sonar/python/checks/FastAPIBindToAllNetworkInterfacesCheck.java renamed to python-checks/src/main/java/org/sonar/python/checks/BindToAllNetworkInterfacesCheck.java python-checks/src/main/java/org/sonar/python/checks/FastAPIBindToAllNetworkInterfacesCheck.java renamed to python-checks/src/main/java/org/sonar/python/checks/BindToAllNetworkInterfacesCheck.java

@@ -16,6 +16,7 @@
  */
 package org.sonar.python.checks;
 
+import javax.annotation.Nullable;
 import org.sonar.check.Rule;
 import org.sonar.plugins.python.api.PythonSubscriptionCheck;
 import org.sonar.plugins.python.api.SubscriptionContext;
@@ -29,30 +30,36 @@
 import org.sonar.python.tree.TreeUtils;
 
 @Rule(key = "S8392")
-public class FastAPIBindToAllNetworkInterfacesCheck extends PythonSubscriptionCheck {
+public class BindToAllNetworkInterfacesCheck extends PythonSubscriptionCheck {
 
   private static final String ALL_NETWORK_INTERFACES = "0.0.0.0";
-  private static final TypeMatcher UVICORN_RUN_FUNCTION_TYPE_MATCHER = TypeMatchers.isType("uvicorn.run");
+  private static final String MESSAGE = "Avoid binding the application to all network interfaces.";
+  private static final TypeMatcher UVICORN_APP_RUN_TYPE_MATCHER = TypeMatchers.isType("uvicorn.run");
+  private static final TypeMatcher FLASK_APP_RUN_TYPE_MATCHER = TypeMatchers.isType("flask.app.Flask.run");
 
   @Override
   public void initialize(Context context) {
-    context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, FastAPIBindToAllNetworkInterfacesCheck::checkUvicornRunFunctionCalls);
+    context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, BindToAllNetworkInterfacesCheck::checkFunctionCalls);
   }
-
-  private static void checkUvicornRunFunctionCalls(SubscriptionContext ctx) {
+  private static void checkFunctionCalls(SubscriptionContext ctx) {
     CallExpression callExpr = ((CallExpression) ctx.syntaxNode());
-
-    if (!UVICORN_RUN_FUNCTION_TYPE_MATCHER.isTrueFor(callExpr.callee(), ctx)) {
-      return;
+    RegularArgument hostArgument = null;
+    if (UVICORN_APP_RUN_TYPE_MATCHER.isTrueFor(callExpr.callee(), ctx)) {
+      hostArgument = TreeUtils.argumentByKeyword("host", callExpr.arguments());
+    } else if (FLASK_APP_RUN_TYPE_MATCHER.isTrueFor(callExpr.callee(), ctx)) {
+      hostArgument = TreeUtils.nthArgumentOrKeyword(0, "host", callExpr.arguments());
+    }
+    if (isHostBoundToAll(hostArgument)) {
+      ctx.addIssue(callExpr.callee(), MESSAGE);
     }
+  }
 
-    RegularArgument hostArgument = TreeUtils.argumentByKeyword("host", callExpr.arguments());
-    if (hostArgument == null) {
-      return;
+  private static boolean isHostBoundToAll(@Nullable RegularArgument hostArgument) {
+    if (hostArgument==null){
+      return false;
     }
     StringLiteral hostValue = Expressions.extractStringLiteral(hostArgument.expression());
-    if (hostValue != null && ALL_NETWORK_INTERFACES.equals(hostValue.trimmedQuotesValue())) {
-      ctx.addIssue(hostArgument, "Avoid binding the FastAPI application to all network interfaces.");
-    }
+    return hostValue != null && ALL_NETWORK_INTERFACES.equals(hostValue.trimmedQuotesValue());
   }
 }
+

python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java

@@ -143,6 +143,7 @@ public Stream<Class<?>> getChecks() {
       BackslashInStringCheck.class,
       BackticksUsageCheck.class,
       BareRaiseInFinallyCheck.class,
+      BindToAllNetworkInterfacesCheck.class,
       BooleanExpressionInExceptCheck.class,
       BooleanCheckNotInvertedCheck.class,
       BreakContinueOutsideLoopCheck.class,
@@ -218,7 +219,6 @@ public Stream<Class<?>> getChecks() {
       ExecStatementUsageCheck.class,
       ExitHasBadArgumentsCheck.class,
       ExpandingArchiveCheck.class,
-      FastAPIBindToAllNetworkInterfacesCheck.class,
       FastAPIDependencyAnnotatedCheck.class,
       FastAPIFileUploadFormCheck.class,
       FastAPIGenericRouteDecoratorCheck.class,

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S8392.html

@@ -1,8 +1,8 @@
-<p>This is an issue when a FastAPI application uses <code>uvicorn.run()</code> with <code>host="0.0.0.0"</code>, which binds the application to all
-available network interfaces on the host machine.</p>
+<p>This is an issue when a web server uses <code>host="0.0.0.0"</code>, which binds the application to all available network interfaces on the host
+machine.</p>
 <h2>Why is this an issue?</h2>
-<p>When you start a FastAPI application, you need to specify which network interface it should listen on. A network interface is a connection point
-between your computer and a network.</p>
+<p>When you start a Web server, you need to specify which network interface it should listen on. A network interface is a connection point between
+your computer and a network.</p>
 <p>The special IP address <code>0.0.0.0</code> tells the application to bind to <strong>all</strong> network interfaces on the machine. This means the
 application becomes accessible from:</p>
 <ul>
@@ -43,7 +43,46 @@ <h3>What is the potential impact?</h3>
 </ul>
 <p>The severity depends on the environment and what security controls are in place, but the risk is highest in development environments where security
 measures are typically minimal.</p>
-<h2>How to fix it</h2>
+<h2>How to fix it in Flask</h2>
+<p>For development, bind to localhost only. For production, use a proper WSGI server instead of app.run().</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+from flask import Flask
+app = Flask(__name__)
+@app.route('/')
+def hello():
+    return 'Hello World!'
+if __name__ == '__main__':
+    app.run(host='0.0.0.0', debug=True)  # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+from flask import Flask
+app = Flask(__name__)
+@app.route('/')
+def hello():
+    return 'Hello World!'
+if __name__ == '__main__':
+    app.run(host='0.0.0.0', debug=False)
+</pre>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="2" data-diff-type="noncompliant">
+import os
+from flask import Flask
+app = Flask(__name__)
+if __name__ == '__main__':
+    app.run(host='0.0.0.0', port=5000, debug=True)  # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="2" data-diff-type="compliant">
+import os
+from flask import Flask
+app = Flask(__name__)
+if __name__ == '__main__':
+    app.run(host='127.0.0.1', port=5000, debug=True)
+</pre>
+<h2>How to fix it in FastAPI</h2>
 <p>For development and local testing, bind to localhost (<code>127.0.0.1</code> or <code>localhost</code>) instead of all interfaces. This ensures the
 application is only accessible from your local machine.</p>
 <h3>Code examples</h3>
@@ -76,6 +115,11 @@ <h3>Documentation</h3>
   options</a> </li>
   <li> FastAPI CLI Documentation - <a href="https://fastapi.tiangolo.com/fastapi-cli/">Documentation explaining the difference between development
   (127.0.0.1) and production (0.0.0.0) binding</a> </li>
+  <li> Flask Deployment Options - <a href="https://flask.palletsprojects.com/en/2.3.x/deploying/">Official Flask documentation on secure deployment
+  practices</a> </li>
+  <li> Flask Security Considerations - <a href="https://flask.palletsprojects.com/en/2.3.x/security/">Flask security best practices and common
+  vulnerabilities</a> </li>
+  <li> Gunicorn Documentation - <a href="https://docs.gunicorn.org/en/stable/">Production WSGI server for Python web applications</a> </li>
 </ul>
 <h3>Standards</h3>
 <ul>

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S8392.json

@@ -1,5 +1,5 @@
 {
-  "title": "FastAPI applications should not bind to all network interfaces",
+  "title": "Web servers should not bind to all network interfaces",
   "type": "VULNERABILITY",
   "status": "ready",
   "remediation": {

…BindToAllNetworkInterfacesCheckTest.java …BindToAllNetworkInterfacesCheckTest.javapython-checks/src/test/java/org/sonar/python/checks/FastAPIBindToAllNetworkInterfacesCheckTest.java renamed to python-checks/src/test/java/org/sonar/python/checks/BindToAllNetworkInterfacesCheckTest.java python-checks/src/test/java/org/sonar/python/checks/FastAPIBindToAllNetworkInterfacesCheckTest.java renamed to python-checks/src/test/java/org/sonar/python/checks/BindToAllNetworkInterfacesCheckTest.java

@@ -19,11 +19,11 @@
 import org.junit.jupiter.api.Test;
 import org.sonar.python.checks.utils.PythonCheckVerifier;
 
-class FastAPIBindToAllNetworkInterfacesCheckTest {
+class BindToAllNetworkInterfacesCheckTest {
 
   @Test
   void test() {
-    PythonCheckVerifier.verify("src/test/resources/checks/fastAPIBindToAllNetworkInterfaces.py", new FastAPIBindToAllNetworkInterfacesCheck());
+    PythonCheckVerifier.verify("src/test/resources/checks/bindToAllNetworkInterfaces.py", new BindToAllNetworkInterfacesCheck());
   }
 
 }

python-checks/src/test/resources/checks/bindToAllNetworkInterfaces.py

@@ -0,0 +1,44 @@
+import uvicorn
+from fastapi import FastAPI
+from flask import Flask
+from somewhere import my_host_name
+
+def fast_api_tests():
+    app_fastapi = FastAPI()
+
+    uvicorn.run(app_fastapi, host="127.0.0.1")
+    uvicorn.run(app_fastapi, port=8000)
+    uvicorn.run(app_fastapi, host="127.0.0.1", port=8000)
+    uvicorn.run(app_fastapi, host="localhost", port=8000)
+    uvicorn.run(app_fastapi, host="192.168.1.100", port=8000)
+    uvicorn.run(app_fastapi, host=my_host_name, port=8000)
+
+    uvicorn.run(app_fastapi, host="0.0.0.0") # Noncompliant {{Avoid binding the application to all network interfaces.}}
+#   ^^^^^^^^^^^
+
+    uvicorn.run(app_fastapi, host="0.0.0.0", port=8000) # Noncompliant
+    uvicorn.run(app_fastapi, host="0.0.0.0", port=8000, debug=True) # Noncompliant
+    host_config = "0.0.0.0"
+    uvicorn.run(app_fastapi, host=host_config, port=8000) # Noncompliant
+
+def flask_tests():
+    app_flask = Flask(__name__)
+
+    app_flask.run(host='0.0.0.0', debug=True)  # Noncompliant {{Avoid binding the application to all network interfaces.}}
+#   ^^^^^^^^^^^^^
+    app_flask.run('0.0.0.0', 5000, True)  # Noncompliant
+    app_flask.run(host='0.0.0.0', debug=False) # Noncompliant
+    app_flask.run(host='0.0.0.0') # Noncompliant
+    app_flask.run(host='127.0.0.1', debug=True)
+    app_flask.run(host='', debug=True)
+    app_flask.run('', debug=True)
+    app_flask.run(None, debug=True)
+    debug = True
+    host = '0.0.0.0'
+    app_flask.run(host=host, debug=debug) # Noncompliant
+    other_host = '12'
+    app_flask.run(other_host, debug=debug)
+    app_flask.run() # Incorrect syntax
+    reassigned = '0.0.0.0'
+    app_flask.run(reassigned, debug=True) # FN limitation of singleAssignedValue
+    reassigned = '0.0.0.0'