|
| 1 | +/** |
| 2 | + * @name Unsecure basic authentication |
| 3 | + * @description Basic authentication only obfuscates username/password in Base64 encoding, which can be easily recognized and reversed. Transmission of sensitive information not in HTTPS is vulnerable to packet sniffing. |
| 4 | + * @kind problem |
| 5 | + * @id java/unsecure-basic-auth |
| 6 | + * @tags security |
| 7 | + * external/cwe-522 |
| 8 | + * external/cwe-319 |
| 9 | + */ |
| 10 | + |
| 11 | +import java |
| 12 | +import semmle.code.java.frameworks.Networking |
| 13 | +import semmle.code.java.dataflow.TaintTracking |
| 14 | +import DataFlow::PathGraph |
| 15 | + |
| 16 | +/** |
| 17 | + * The Java class `org.apache.http.message.AbstractHttpMessage`. Popular subclasses include `HttpGet`, `HttpPost`, and `HttpPut`. |
| 18 | + */ |
| 19 | +class ApacheHttpRequest extends RefType { |
| 20 | + ApacheHttpRequest() { hasQualifiedName("org.apache.http.message", "AbstractHttpMessage") } |
| 21 | +} |
| 22 | + |
| 23 | +/** |
| 24 | + * Class of Java URL constructor |
| 25 | + */ |
| 26 | +class URLConstructor extends ClassInstanceExpr { |
| 27 | + URLConstructor() { this.getConstructor().getDeclaringType() instanceof TypeUrl } |
| 28 | + |
| 29 | + Expr stringArg() { |
| 30 | + // Query only in URL's that were constructed by calling the single parameter string constructor. |
| 31 | + this.getConstructor().getNumberOfParameters() = 1 and |
| 32 | + this.getConstructor().getParameter(0).getType() instanceof TypeString and |
| 33 | + result = this.getArgument(0) |
| 34 | + } |
| 35 | +} |
| 36 | + |
| 37 | +/** |
| 38 | + * The type `java.net.URLConnection`. |
| 39 | + */ |
| 40 | +class TypeHttpUrlConnection extends RefType { |
| 41 | + TypeHttpUrlConnection() { hasQualifiedName("java.net", "HttpURLConnection") } |
| 42 | +} |
| 43 | + |
| 44 | +/** |
| 45 | + * String of HTTP URLs not in private domains |
| 46 | + */ |
| 47 | +class HttpString extends StringLiteral { |
| 48 | + HttpString() { |
| 49 | + // Match URLs with the HTTP protocol and without private IP addresses to reduce false positives. |
| 50 | + exists(string s | this.getRepresentedString() = s | |
| 51 | + s.matches("http://%") and |
| 52 | + not s.matches("%/localhost%") and |
| 53 | + not s.matches("%/127.0.0.1%") and |
| 54 | + not s.matches("%/10.%") and |
| 55 | + not s.matches("%/172.16.%") and |
| 56 | + not s.matches("%/192.168.%") |
| 57 | + ) |
| 58 | + } |
| 59 | +} |
| 60 | + |
| 61 | +/** |
| 62 | + * String concatenated with `HttpString` |
| 63 | + */ |
| 64 | +predicate builtFromHttpStringConcat(Expr expr) { |
| 65 | + expr instanceof HttpString |
| 66 | + or |
| 67 | + exists(AddExpr concatExpr | concatExpr = expr | |
| 68 | + builtFromHttpStringConcat(concatExpr.getLeftOperand()) |
| 69 | + ) |
| 70 | + or |
| 71 | + exists(AddExpr concatExpr | concatExpr = expr | |
| 72 | + exists(Expr arg | arg = concatExpr.getLeftOperand() | arg instanceof HttpString) |
| 73 | + ) |
| 74 | + or |
| 75 | + exists(Expr other | builtFromHttpStringConcat(other) | |
| 76 | + exists(Variable var | var.getAnAssignedValue() = other and var.getAnAccess() = expr) |
| 77 | + ) |
| 78 | +} |
| 79 | + |
| 80 | +/** |
| 81 | + * The methods `addHeader()` and `setHeader` declared in ApacheHttpRequest invoked for basic authentication. |
| 82 | + */ |
| 83 | +class AddBasicAuthHeaderMethodAccess extends MethodAccess { |
| 84 | + AddBasicAuthHeaderMethodAccess() { |
| 85 | + this.getMethod().getDeclaringType() instanceof ApacheHttpRequest and |
| 86 | + (this.getMethod().hasName("addHeader") or this.getMethod().hasName("setHeader")) and |
| 87 | + this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "Authorization" and |
| 88 | + exists(Expr arg1 | |
| 89 | + arg1 = this.getArgument(1) and //Check three patterns |
| 90 | + ( |
| 91 | + arg1 //String authStringEnc = "Basic ...."; post.addHeader("Authorization", authStringEnc) |
| 92 | + .(VarAccess) |
| 93 | + .getVariable() |
| 94 | + .getAnAssignedValue() |
| 95 | + .(StringLiteral) |
| 96 | + .getRepresentedString() |
| 97 | + .matches("Basic %") or |
| 98 | + arg1.(CompileTimeConstantExpr).getStringValue().matches("Basic %") or //post.addHeader("Authorization", "Basic ....") |
| 99 | + arg1.(AddExpr).getLeftOperand().(StringLiteral).getRepresentedString().matches("Basic %") //post.addHeader("Authorization", "Basic "+authStringEnc) |
| 100 | + ) |
| 101 | + ) and |
| 102 | + exists(VarAccess request, VariableAssign va, ConstructorCall cc, Expr arg0 | |
| 103 | + this.getQualifier() = request and //Check the method invocation with the pattern post.addHeader("Authorization", "Basic " + authStringEnc) |
| 104 | + va.getDestVar() = request.getVariable() and |
| 105 | + va.getSource() = cc and |
| 106 | + cc.getArgument(0) = arg0 and |
| 107 | + builtFromHttpStringConcat(arg0) // Check url string |
| 108 | + ) |
| 109 | + } |
| 110 | +} |
| 111 | + |
| 112 | +/** The `openConnection` method of Java URL. Not to include `openStream` since it won't be used in this query. */ |
| 113 | +class HttpURLOpenMethod extends Method { |
| 114 | + HttpURLOpenMethod() { |
| 115 | + this.getDeclaringType() instanceof TypeUrl and |
| 116 | + this.getName() = "openConnection" |
| 117 | + } |
| 118 | +} |
| 119 | + |
| 120 | +/** |
| 121 | + * Tracks the flow of data from parameter of URL constructor to the url instance |
| 122 | + */ |
| 123 | +class URLConstructorTaintStep extends TaintTracking::AdditionalTaintStep { |
| 124 | + override predicate step(DataFlow::Node node1, DataFlow::Node node2) { |
| 125 | + exists(URLConstructor u | |
| 126 | + node1.asExpr() = u.stringArg() and |
| 127 | + node2.asExpr() = u |
| 128 | + ) |
| 129 | + } |
| 130 | +} |
| 131 | + |
| 132 | +/** |
| 133 | + * Tracks the flow of data from `openConnection` method to the connection object |
| 134 | + */ |
| 135 | +class OpenHttpURLTaintStep extends TaintTracking::AdditionalTaintStep { |
| 136 | + override predicate step(DataFlow::Node node1, DataFlow::Node node2) { |
| 137 | + exists(MethodAccess ma, VariableAssign va | |
| 138 | + ma.getMethod() instanceof HttpURLOpenMethod and |
| 139 | + ma.getQualifier() = node1.asExpr() and |
| 140 | + ( |
| 141 | + ma = va.getSource() |
| 142 | + or |
| 143 | + exists(CastExpr ce | |
| 144 | + ce.getExpr() = ma and |
| 145 | + ce = va.getSource() and |
| 146 | + ce.getControlFlowNode().getASuccessor() = va //With a type cast like HttpURLConnection conn = (HttpURLConnection) url.openConnection(); |
| 147 | + ) |
| 148 | + ) and |
| 149 | + node2.asExpr() = va.getDestVar().getAnAccess() |
| 150 | + ) |
| 151 | + } |
| 152 | +} |
| 153 | + |
| 154 | +class HttpStringToHttpURLOpenMethodFlowConfig extends TaintTracking::Configuration { |
| 155 | + HttpStringToHttpURLOpenMethodFlowConfig() { |
| 156 | + this = "UnsecureBasicAuth::HttpStringToHttpURLOpenMethodFlowConfig" |
| 157 | + } |
| 158 | + |
| 159 | + override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof HttpString } |
| 160 | + |
| 161 | + override predicate isSink(DataFlow::Node sink) { |
| 162 | + sink.asExpr().(VarAccess).getVariable().getType() instanceof TypeUrlConnection or |
| 163 | + sink.asExpr().(VarAccess).getVariable().getType() instanceof TypeHttpUrlConnection // Somehow TypeHttpUrlConnection isn't instance of TypeUrlConnection |
| 164 | + } |
| 165 | + |
| 166 | + override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { |
| 167 | + exists(DataFlow::Node nodem | |
| 168 | + any(URLConstructorTaintStep uts).step(node1, nodem) and |
| 169 | + any(OpenHttpURLTaintStep ots).step(nodem, node2) |
| 170 | + ) |
| 171 | + } |
| 172 | + |
| 173 | + override predicate isSanitizer(DataFlow::Node node) { |
| 174 | + node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType |
| 175 | + } |
| 176 | +} |
| 177 | + |
| 178 | +/** |
| 179 | + * The method `setRequestProperty()` declared in URL Connection invoked for basic authentication. |
| 180 | + */ |
| 181 | +class SetBasicAuthPropertyMethodAccess extends MethodAccess { |
| 182 | + SetBasicAuthPropertyMethodAccess() { |
| 183 | + this.getMethod().getDeclaringType() instanceof TypeUrlConnection and |
| 184 | + this.getMethod().hasName("setRequestProperty") and |
| 185 | + this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "Authorization" and |
| 186 | + exists(Expr arg1 | |
| 187 | + arg1 = this.getArgument(1) and //Check three patterns |
| 188 | + ( |
| 189 | + arg1 //String authStringEnc = "Basic ...."; conn.setRequestProperty("Authorization", authStringEnc) |
| 190 | + .(VarAccess) |
| 191 | + .getVariable() |
| 192 | + .getAnAssignedValue() |
| 193 | + .(StringLiteral) |
| 194 | + .getRepresentedString() |
| 195 | + .matches("Basic %") or |
| 196 | + arg1.(CompileTimeConstantExpr).getStringValue().matches("Basic %") or //conn.setRequestProperty("Authorization", "Basic ....") |
| 197 | + arg1.(AddExpr).getLeftOperand().(StringLiteral).getRepresentedString().matches("Basic %") //conn.setRequestProperty("Authorization", "Basic "+authStringEnc) |
| 198 | + ) |
| 199 | + ) and |
| 200 | + exists(VarAccess conn, DataFlow::PathNode source, DataFlow::PathNode sink, HttpString s | |
| 201 | + this.getQualifier() = conn and //HttpURLConnection conn = (HttpURLConnection) url.openConnection(); |
| 202 | + source.getNode().asExpr() = s and |
| 203 | + sink.getNode().asExpr() = conn.getVariable().getAnAccess() and |
| 204 | + any(HttpStringToHttpURLOpenMethodFlowConfig c).hasFlowPath(source, sink) |
| 205 | + ) |
| 206 | + } |
| 207 | +} |
| 208 | + |
| 209 | +from MethodAccess ma |
| 210 | +where |
| 211 | + ma instanceof AddBasicAuthHeaderMethodAccess or |
| 212 | + ma instanceof SetBasicAuthPropertyMethodAccess |
| 213 | +select ma, "Unsafe basic authentication" |
0 commit comments