From 7f980a4901452f184da21200a6e4eda8bd16d886 Mon Sep 17 00:00:00 2001
From: dilanbhalla <dbhalla21@berkeley.edu>
Date: Fri, 14 Aug 2020 00:45:08 -0700
Subject: [PATCH] pr fixes

---
 .../experimental/CWE-327/CryptoLibraries.qll  | 66 +++++++------------
 .../CWE-327/WeakCryptoAlgorithm.ql            |  4 +-
 2 files changed, 27 insertions(+), 43 deletions(-)

diff --git a/ql/src/experimental/CWE-327/CryptoLibraries.qll b/ql/src/experimental/CWE-327/CryptoLibraries.qll
index 4233e42201b..994e6a72f82 100644
--- a/ql/src/experimental/CWE-327/CryptoLibraries.qll
+++ b/ql/src/experimental/CWE-327/CryptoLibraries.qll
@@ -127,7 +127,6 @@ abstract class CryptographicAlgorithm extends TCryptographicAlgorithm {
   bindingset[name]
   predicate matchesName(string name) {
     exists(name.regexpReplaceAll("[-_]", "").regexpFind("(?i)\\Q" + getName() + "\\E", _, _))
-    // name.toUpperCase().regexpReplaceAll("[-_ ]", "").regexpMatch(".*" + getName() + ".*")
   }
 
   /**
@@ -194,70 +193,53 @@ abstract class CryptographicOperation extends DataFlow::Node {
 }
 
 /**
- * Below are the cryptographic functions that have been implemented so far for this library.
- * Class that checks for use of Md5 package.
+ * Models cryptographic operations of the `crypto/md5` package.
  */
 class Md5 extends CryptographicOperation, DataFlow::CallNode {
-  Expr input;
-  CryptographicAlgorithm algorithm;
+  Md5() { getTarget().hasQualifiedName("crypto/md5", ["New", "Sum"]) }
 
-  Md5() {
-    getTarget().hasQualifiedName("crypto/md5", ["New", "Sum"]) and
-    this.getArgument(0).asExpr() = input
+  override Expr getInput() { result = this.getArgument(0).asExpr() }
+
+  override CryptographicAlgorithm getAlgorithm() {
+    result.matchesName(this.getTarget().getPackage().getName())
   }
-
-  override Expr getInput() { result = input }
-
-  override CryptographicAlgorithm getAlgorithm() { result = algorithm }
 }
 
 /**
- * Class that checks for use of Sha1 package.
+ * Models cryptographic operations of the `crypto/sha1` package.
  */
 class Sha1 extends CryptographicOperation, DataFlow::CallNode {
-  Expr input;
-  CryptographicAlgorithm algorithm;
+  Sha1() { getTarget().hasQualifiedName("crypto/sha1", ["New", "Sum"]) }
 
-  Sha1() {
-    getTarget().hasQualifiedName("crypto/sha1", ["New", "Sum"]) and
-    this.getArgument(0).asExpr() = input
+  override Expr getInput() { result = this.getArgument(0).asExpr() }
+
+  override CryptographicAlgorithm getAlgorithm() {
+    result.matchesName(this.getTarget().getPackage().getName())
   }
-
-  override Expr getInput() { result = input }
-
-  override CryptographicAlgorithm getAlgorithm() { result = algorithm }
 }
 
 /**
- * Class that checks for use of Des package.
+ * Models cryptographic operations of the `crypto/des` package.
  */
 class Des extends CryptographicOperation, DataFlow::CallNode {
-  Expr input;
-  CryptographicAlgorithm algorithm;
+  Des() { getTarget().hasQualifiedName("crypto/des", ["NewCipher", "NewTripleDESCipher"]) }
 
-  Des() {
-    getTarget().hasQualifiedName("crypto/des", ["NewCipher", "NewTripleDESCipher"]) and
-    this.getArgument(0).asExpr() = input
+  override Expr getInput() { result = this.getArgument(0).asExpr() }
+
+  override CryptographicAlgorithm getAlgorithm() {
+    result.matchesName(this.getTarget().getPackage().getName())
   }
-
-  override Expr getInput() { result = input }
-
-  override CryptographicAlgorithm getAlgorithm() { result = algorithm }
 }
 
 /**
- * Class that checks for use of Rc4 package.
+ * Models cryptographic operations of the `crypto/rc4` package.
  */
 class Rc4 extends CryptographicOperation, DataFlow::CallNode {
-  Expr input;
-  CryptographicAlgorithm algorithm;
+  Rc4() { getTarget().hasQualifiedName("crypto/rc4", ["NewCipher"]) }
 
-  Rc4() {
-    getTarget().hasQualifiedName("crypto/rc4", ["NewCipher"]) and
-    this.getArgument(0).asExpr() = input
+  override Expr getInput() { result = this.getArgument(0).asExpr() }
+
+  override CryptographicAlgorithm getAlgorithm() {
+    result.matchesName(this.getTarget().getPackage().getName())
   }
-
-  override Expr getInput() { result = input }
-
-  override CryptographicAlgorithm getAlgorithm() { result = algorithm }
 }
diff --git a/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.ql b/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.ql
index 1d24d0e40b6..6cec0620eb0 100644
--- a/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.ql
+++ b/ql/src/experimental/CWE-327/WeakCryptoAlgorithm.ql
@@ -5,6 +5,7 @@
  * @problem.severity error
  * @id go/weak-crypto-algorithm
  * @tags security
+ *         external/cwe/cwe-327
  */
 
 import go
@@ -13,4 +14,5 @@ import DataFlow::PathGraph
 
 from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
 where cfg.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "Sensitive data is used in a weak cryptographic algorithm."
+select sink.getNode(), source, sink, "$@ is used in a weak cryptographic algorithm.",
+  source.getNode(), "Sensitive data"