Skip to content

Commit

Permalink
SONARPY-1475 Rule S5332: Support http.server.HTTPServer and subclas…
Browse files Browse the repository at this point in the history 
…ses (#1635)
  • Loading branch information
@joke1196
joke1196 authored Nov 8, 2023
1 parent 58ad17d commit 49777dd
Show file tree
Hide file tree
Showing 2 changed files with 148 additions and 13 deletions.
97 changes: 84 additions & 13 deletions python-checks/src/main/java/org/sonar/python/checks/hotspots/ClearTextProtocolsCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,27 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
import java.util.stream.Stream;

import org.sonar.check.Rule;
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
import org.sonar.plugins.python.api.SubscriptionContext;
import org.sonar.plugins.python.api.symbols.Symbol;
import org.sonar.plugins.python.api.tree.ArgList;
import org.sonar.plugins.python.api.tree.Argument;
import org.sonar.plugins.python.api.tree.AssignmentStatement;
import org.sonar.plugins.python.api.tree.CallExpression;
import org.sonar.plugins.python.api.tree.ClassDef;
import org.sonar.plugins.python.api.tree.Expression;
import org.sonar.plugins.python.api.tree.FunctionDef;
import org.sonar.plugins.python.api.tree.HasSymbol;
import org.sonar.plugins.python.api.tree.Name;
import org.sonar.plugins.python.api.tree.QualifiedExpression;
import org.sonar.plugins.python.api.tree.RegularArgument;
import org.sonar.plugins.python.api.tree.StringElement;
import org.sonar.plugins.python.api.tree.Tree;
import org.sonar.python.checks.Expressions;
Expand All @@ -47,14 +57,16 @@ public class ClearTextProtocolsCheck extends PythonSubscriptionCheck {
private static final List<String> SENSITIVE_PROTOCOLS = Arrays.asList("http://", "ftp://", "telnet://");
private static final Pattern LOOPBACK = Pattern.compile("localhost|127(?:\\.[0-9]+){0,2}\\.[0-9]+$|^(?:0*\\:)*?:?0*1", Pattern.CASE_INSENSITIVE);
private static final Map<String, String> ALTERNATIVES = new HashMap<>();
private static final String SENSITIVE_HTTP_SERVER_CALL = "socketserver.BaseServer.serve_forever";
private static final Set<String> SENSITIVE_HTTP_SERVER_CLASSES = Set.of("http.server.HTTPServer", "http.server.ThreadingHTTPServer");


static {
ALTERNATIVES.put("http", "https");
ALTERNATIVES.put("ftp", "sftp, scp or ftps");
ALTERNATIVES.put("telnet", "ssh");
}


@Override
public void initialize(Context context) {
context.registerSyntaxNodeConsumer(Tree.Kind.STRING_ELEMENT, ctx -> {
Expand All @@ -66,15 +78,74 @@ public void initialize(Context context) {
.ifPresent(protocol -> ctx.addIssue(node, message(protocol)));
});
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, ctx -> {
Symbol symbol = ((CallExpression) ctx.syntaxNode()).calleeSymbol();
isUnsafeLib(symbol).ifPresent(protocol -> ctx.addIssue(ctx.syntaxNode(), message(protocol)));
CallExpression callExpression = (CallExpression) ctx.syntaxNode();
Optional.ofNullable(callExpression.calleeSymbol())
.map(Symbol::fullyQualifiedName)
.flatMap(ClearTextProtocolsCheck::isUnsafeLib)
.ifPresent(protocol -> ctx.addIssue(callExpression, message(protocol)));
});

context.registerSyntaxNodeConsumer(Tree.Kind.ASSIGNMENT_STMT, ctx -> handleAssignmentStatement((AssignmentStatement) ctx.syntaxNode(), ctx));

context.registerSyntaxNodeConsumer(Tree.Kind.QUALIFIED_EXPR, ClearTextProtocolsCheck::checkServerCallFromSuper);

context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, ClearTextProtocolsCheck::checkServerBindOverrides);
new ClearTextProtocolsCheckPart().initialize(context);
}

private static void checkServerCallFromSuper(SubscriptionContext ctx) {
QualifiedExpression qualifiedExpression = (QualifiedExpression) ctx.syntaxNode();
Optional.of(qualifiedExpression)
.filter(qe -> isName("serve_forever", qe.name()) && isCallToSensitiveSuperClass(qe))
.map(qe -> TreeUtils.firstAncestorOfKind(qe, Tree.Kind.CALL_EXPR))
.flatMap(TreeUtils.toOptionalInstanceOfMapper(CallExpression.class))
.ifPresent(ce -> ctx.addIssue(ce, message("http")));
}

private static void checkServerBindOverrides(SubscriptionContext ctx) {
FunctionDef funcDef = (FunctionDef) ctx.syntaxNode();
Optional.of(funcDef)
.filter(fd -> isName("server_bind", fd.name()) && isParentClassExtendingSensitiveClass(funcDef))
.ifPresent(fd -> ctx.addIssue(fd.defKeyword(), fd.rightPar(), message("http")));
}

private static boolean isName(String nameToCheck, Name name) {
return nameToCheck.equals(name.name());
}

private static boolean isCallToSensitiveSuperClass(QualifiedExpression expression) {
return Optional.of(expression.qualifier())
.flatMap(TreeUtils.toOptionalInstanceOfMapper(CallExpression.class))
.map(CallExpression::callee)
.flatMap(TreeUtils.toOptionalInstanceOfMapper(Name.class))
.map(Name::name)
.filter("super"::equals)
.filter(name -> isParentClassExtendingSensitiveClass(expression))
.isPresent();
}

private static boolean isParentClassExtendingSensitiveClass(Tree expression) {
return Optional.ofNullable(TreeUtils.firstAncestorOfKind(expression, Tree.Kind.CLASSDEF))
.map(ClassDef.class::cast)
.map(ClassDef::args)
.map(ArgList::arguments)
.map(ClearTextProtocolsCheck::getClassFQNFromArgument)
.map(arguments -> arguments.anyMatch(SENSITIVE_HTTP_SERVER_CLASSES::contains))
.orElse(false);
}

public static Stream<String> getClassFQNFromArgument(List<Argument> arguments) {
return arguments.stream()
.map(TreeUtils.toInstanceOfMapper(RegularArgument.class))
.filter(Objects::nonNull)
.map(RegularArgument::expression)
.map(TreeUtils.toInstanceOfMapper(Name.class))
.filter(Objects::nonNull)
.map(Name::symbol)
.filter(Objects::nonNull)
.map(Symbol::fullyQualifiedName);
}

private static void handleAssignmentStatement(AssignmentStatement assignmentStatement, SubscriptionContext ctx) {
if (assignmentStatement.lhsExpressions().size() > 1) {
// avoid potential FPs
Expand Down Expand Up @@ -127,15 +198,15 @@ private static Optional<String> unsafeProtocol(String literalValue) {
return Optional.empty();
}

private static Optional<String> isUnsafeLib(@Nullable Symbol symbol) {
if (symbol != null) {
String qualifiedName = symbol.fullyQualifiedName();
if ("telnetlib.Telnet".equals(qualifiedName)) {
return Optional.of("telnet");
}
if ("ftplib.FTP".equals(qualifiedName)) {
return Optional.of("ftp");
}
private static Optional<String> isUnsafeLib(String qualifiedName) {
if ("telnetlib.Telnet".equals(qualifiedName)) {
return Optional.of("telnet");
}
if ("ftplib.FTP".equals(qualifiedName)) {
return Optional.of("ftp");
}
if (SENSITIVE_HTTP_SERVER_CALL.equals(qualifiedName)) {
return Optional.of("http");
}
return Optional.empty();
}
Expand Down
64 changes: 64 additions & 0 deletions python-checks/src/test/resources/checks/hotspots/clearTextProtocols.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,67 @@ def FP_starttls_in_different_method():

def start_tls(x):
x.starttls(context=context)


def python_web_server_noncompliant():
from http.server import SimpleHTTPRequestHandler, HTTPServer, ThreadingHTTPServer

http_server = HTTPServer(('0.0.0.0', 8080), SimpleHTTPRequestHandler)
http_server.serve_forever() # Noncompliant
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^
threading_http_server = ThreadingHTTPServer(('0.0.0.0', 8080), SimpleHTTPRequestHandler)
threading_http_server.serve_forever() # Noncompliant
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

class MyServer(HTTPServer):
def run(self):
super(self).serve_forever() # Noncompliant
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^

def server_bind(self): # Noncompliant
# ^^^^^^^^^^^^^^^^^^^^^
pass

my_server = MyServer(('0.0.0.0', 8080), SimpleHTTPRequestHandler)
my_server.serve_forever() # Noncompliant
# ^^^^^^^^^^^^^^^^^^^^^^^^^

class MyThreadingServer(ThreadingHTTPServer):
def run(self):
super(self).serve_forever() # Noncompliant
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^

def server_bind(self): # Noncompliant
# ^^^^^^^^^^^^^^^^^^^^^
pass

my_threading_server = MyThreadingServer(('0.0.0.0', 8080), SimpleHTTPRequestHandler)
my_threading_server.serve_forever() # Noncompliant


def python_web_server_compliant(ok_server):

ok_server.serve_forever() # Compliant
ok_server.server_bind()

class HTTPServer():
def serve_forever(self):
pass

def server_bind(self):
pass

class MyServer(HTTPServer):
def run(self):
super(self).serve_forever() # Compliant

def server_bind(self):
pass

my_server = MyServer()
my_server.serve_forever()
my_server.server_bind()

my_server = HTTPServer()
my_server.serve_forever()
my_server.server_bind()

0 comments on commit 49777dd

Please sign in to comment.