diff --git a/csharp/ql/src/Bad Practices/Naming Conventions/ConfusingOverridesNames.ql b/csharp/ql/src/Bad Practices/Naming Conventions/ConfusingOverridesNames.ql index 173a8220430..c1a3e5612b6 100644 --- a/csharp/ql/src/Bad Practices/Naming Conventions/ConfusingOverridesNames.ql +++ b/csharp/ql/src/Bad Practices/Naming Conventions/ConfusingOverridesNames.ql @@ -43,19 +43,16 @@ predicate confusing(Method m1, Method m2) { ) } -/* - * Two method names are confusing if all of the following conditions hold: - * They are both static methods or both instance methods. - * They are not declared in the same class, and the superclass method is - * not overridden in an intermediate class - * They have different names. - * They have the same names if case is ignored. - * There is no method in the subclass that has the same name as - * the superclass method - * There is an additional check that only methods with names longer than one character - * can be considered confusing. - */ - +// Two method names are confusing if all of the following conditions hold: +// They are both static methods or both instance methods. +// They are not declared in the same class, and the superclass method is +// not overridden in an intermediate class +// They have different names. +// They have the same names if case is ignored. +// There is no method in the subclass that has the same name as +// the superclass method +// There is an additional check that only methods with names longer than one character +// can be considered confusing. from Method m1, Method m2 where confusing(m1, m2) and diff --git a/csharp/ql/src/Metrics/Files/FSelfContainedness.ql b/csharp/ql/src/Metrics/Files/FSelfContainedness.ql index 6763d4881f2..a057479a85c 100644 --- a/csharp/ql/src/Metrics/Files/FSelfContainedness.ql +++ b/csharp/ql/src/Metrics/Files/FSelfContainedness.ql @@ -13,7 +13,7 @@ import csharp import semmle.code.csharp.metrics.Coupling -/* Self-containedness on file level */ +// Self-containedness on file level from File f, float selfContaindness, int efferentSourceCoupling, int efferentCoupling where efferentSourceCoupling = count(File g | diff --git a/csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql b/csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql index 816503785f0..134031adf43 100644 --- a/csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql +++ b/csharp/ql/src/Security Features/CWE-352/MissingAntiForgeryTokenValidation.ql @@ -26,10 +26,7 @@ class AntiForgeryAuthorizationFilter extends AuthorizationFilter { * Holds if the project has a global anti forgery filter. */ predicate hasGlobalAntiForgeryFilter() { - /* - * A global filter added - */ - + // A global filter added exists(MethodCall addGlobalFilter | // addGlobalFilter adds a filter to the global filter collection addGlobalFilter.getTarget() = any(GlobalFilterCollection gfc).getAddMethod() and diff --git a/csharp/ql/src/Security Features/CWE-451/MissingXFrameOptions.ql b/csharp/ql/src/Security Features/CWE-451/MissingXFrameOptions.ql index 010ba26a406..5a4774a4dc3 100644 --- a/csharp/ql/src/Security Features/CWE-451/MissingXFrameOptions.ql +++ b/csharp/ql/src/Security Features/CWE-451/MissingXFrameOptions.ql @@ -19,19 +19,16 @@ import semmle.code.csharp.frameworks.system.Web * Holds if there exists a `Web.config` file in the snapshot that adds an `X-Frame-Options` header. */ predicate hasWebConfigXFrameOptions() { - /* - * Looking for an entry in a Web.config file that looks like this: - * ``` - * - * - * - * - * - * - * - * ``` - */ - + // Looking for an entry in a Web.config file that looks like this: + // ``` + // + // + // + // + // + // + // + // ``` exists(XMLElement element | element = any(WebConfigXML webConfig) .getARootElement() diff --git a/csharp/ql/src/Stubs/MinimalStubsFromSource.ql b/csharp/ql/src/Stubs/MinimalStubsFromSource.ql index 116237d8f17..c4311b40ac2 100644 --- a/csharp/ql/src/Stubs/MinimalStubsFromSource.ql +++ b/csharp/ql/src/Stubs/MinimalStubsFromSource.ql @@ -1,4 +1,4 @@ -/* +/** * Tool to generate C# stubs from a qltest snapshot. * * It finds all declarations used in the source code, diff --git a/csharp/ql/src/semmle/code/asp/AspNet.qll b/csharp/ql/src/semmle/code/asp/AspNet.qll index 4447ccfb7d9..268c7076dac 100644 --- a/csharp/ql/src/semmle/code/asp/AspNet.qll +++ b/csharp/ql/src/semmle/code/asp/AspNet.qll @@ -184,15 +184,12 @@ class PageDirective extends AspDirective { ValueOrRefType getInheritedType() { result.getQualifiedName() = getInheritedTypeQualifiedName() } private string getInheritedTypeQualifiedName() { - /* - * Relevant attributes: - * - `Inherits`: determines the class to inherit from, this is what we want - * - `ClassName`: determines the name to use for the compiled class, can - * provide a fallback namespace if `Inherits` does not have one - * - `CodeBehindFile`/`CodeFile`: used by tooling, but not semantically - * relevant at runtime - */ - + // Relevant attributes: + // - `Inherits`: determines the class to inherit from, this is what we want + // - `ClassName`: determines the name to use for the compiled class, can + // provide a fallback namespace if `Inherits` does not have one + // - `CodeBehindFile`/`CodeFile`: used by tooling, but not semantically + // relevant at runtime exists(string inherits | inherits = getAttributeByName("Inherits").getBody() | if inherits.indexOf(".") != -1 then result = inherits diff --git a/csharp/ql/src/semmle/code/csharp/Conversion.qll b/csharp/ql/src/semmle/code/csharp/Conversion.qll index c5329a951f4..63ab98e1675 100644 --- a/csharp/ql/src/semmle/code/csharp/Conversion.qll +++ b/csharp/ql/src/semmle/code/csharp/Conversion.qll @@ -307,24 +307,22 @@ private module Identity { private predicate convIdentityStrictConstructedType( IdentityConvertibleConstructedType fromType, IdentityConvertibleConstructedType toType ) { - /* - * Semantically equivalent with - * ``` - * ugt = fromType.getUnboundGeneric() - * and - * forex(int i | - * i in [0 .. ugt.getNumberOfTypeParameters() - 1] | - * exists(Type t1, Type t2 | - * t1 = getTypeArgument(ugt, fromType, i, _) and - * t2 = getTypeArgument(ugt, toType, i, _) | - * convIdentity(t1, t2) - * ) - * ) - * ``` - * but performance is improved by explicitly evaluating the `i`th argument - * only when all preceding arguments are convertible. - */ - + // Semantically equivalent with + // ``` + // ugt = fromType.getUnboundGeneric() + // and + // forex(int i | + // i in [0 .. ugt.getNumberOfTypeParameters() - 1] | + // exists(Type t1, Type t2 | + // t1 = getTypeArgument(ugt, fromType, i, _) and + // t2 = getTypeArgument(ugt, toType, i, _) | + // convIdentity(t1, t2) + // ) + // ) + // ``` + // but performance is improved by explicitly evaluating the `i`th argument + // only when all preceding arguments are convertible. + // fromType != toType and ( convIdentitySingle(_, fromType, toType) @@ -462,11 +460,8 @@ predicate convNullableType(ValueOrRefType fromType, NullableType toType) { ) } -/* - * This is a deliberate, small Cartesian product, so we have manually lifted it to force the - * evaluator to evaluate it in its entirety, rather than trying to optimize it in context. - */ - +// This is a deliberate, small Cartesian product, so we have manually lifted it to force the +// evaluator to evaluate it in its entirety, rather than trying to optimize it in context. pragma[noinline] private predicate defaultNullConversion(Type fromType, Type toType) { fromType instanceof NullType and convNullType(toType) @@ -491,21 +486,15 @@ private predicate convRefTypeNonNull(Type fromType, Type toType) { convRefTypeParameter(fromType, toType) } -/* - * This is a deliberate, small cartesian product, so we have manually lifted it to force the - * evaluator to evaluate it in its entirety, rather than trying to optimize it in context. - */ - +// This is a deliberate, small cartesian product, so we have manually lifted it to force the +// evaluator to evaluate it in its entirety, rather than trying to optimize it in context. pragma[noinline] private predicate defaultDynamicConversion(Type fromType, Type toType) { fromType instanceof RefType and toType instanceof DynamicType } -/* - * This is a deliberate, small cartesian product, so we have manually lifted it to force the - * evaluator to evaluate it in its entirety, rather than trying to optimize it in context. - */ - +// This is a deliberate, small cartesian product, so we have manually lifted it to force the +// evaluator to evaluate it in its entirety, rather than trying to optimize it in context. pragma[noinline] private predicate defaultDelegateConversion(RefType fromType, RefType toType) { fromType instanceof DelegateType and toType = any(SystemDelegateClass c).getABaseType*() @@ -525,11 +514,8 @@ private predicate convRefTypeRefType(RefType fromType, RefType toType) { ) } -/* - * This is a deliberate, small cartesian product, so we have manually lifted it to force the - * evaluator to evaluate it in its entirety, rather than trying to optimize it in context. - */ - +// This is a deliberate, small cartesian product, so we have manually lifted it to force the +// evaluator to evaluate it in its entirety, rather than trying to optimize it in context. pragma[noinline] private predicate defaultArrayConversion(Type fromType, RefType toType) { fromType instanceof ArrayType and toType = any(SystemArrayClass c).getABaseType*() @@ -745,28 +731,25 @@ predicate convConversionOperator(Type fromType, Type toType) { /** 13.1.3.2: Variance conversion. */ private predicate convVariance(ConstructedType fromType, ConstructedType toType) { - /* - * Semantically equivalent with - * ``` - * ugt = fromType.getUnboundGeneric() - * and - * forex(int i | - * i in [0 .. ugt.getNumberOfTypeParameters() - 1] | - * exists(Type t1, Type t2, TypeParameter tp | - * t1 = getTypeArgument(ugt, fromType, i, tp) and - * t2 = getTypeArgument(ugt, toType, i, tp) | - * convIdentity(t1, t2) - * or - * convRefTypeNonNull(t1, t2) and tp.isOut() - * or - * convRefTypeNonNull(t2, t1) and tp.isIn() - * ) - * ) - * ``` - * but performance is improved by explicitly evaluating the `i`th argument - * only when all preceding arguments are convertible. - */ - + // Semantically equivalent with + // ``` + // ugt = fromType.getUnboundGeneric() + // and + // forex(int i | + // i in [0 .. ugt.getNumberOfTypeParameters() - 1] | + // exists(Type t1, Type t2, TypeParameter tp | + // t1 = getTypeArgument(ugt, fromType, i, tp) and + // t2 = getTypeArgument(ugt, toType, i, tp) | + // convIdentity(t1, t2) + // or + // convRefTypeNonNull(t1, t2) and tp.isOut() + // or + // convRefTypeNonNull(t2, t1) and tp.isIn() + // ) + // ) + // ``` + // but performance is improved by explicitly evaluating the `i`th argument + // only when all preceding arguments are convertible. Variance::convVarianceSingle(_, fromType, toType) or exists(UnboundGenericType ugt | diff --git a/csharp/ql/src/semmle/code/csharp/Enclosing.qll b/csharp/ql/src/semmle/code/csharp/Enclosing.qll index c18a6d2ca12..ab106421196 100644 --- a/csharp/ql/src/semmle/code/csharp/Enclosing.qll +++ b/csharp/ql/src/semmle/code/csharp/Enclosing.qll @@ -27,14 +27,11 @@ module Internal { */ cached predicate enclosingCallable(Stmt s, Callable c) { - /* - * Compute the enclosing callable for a statement. This walks up through - * enclosing statements until it hits a callable. It's unambiguous, since - * if a statement has no parent statement, it's either the method body - * or the body of an anonymous function declaration, in each of which cases the - * non-statement parent is in fact the enclosing callable. - */ - + // Compute the enclosing callable for a statement. This walks up through + // enclosing statements until it hits a callable. It's unambiguous, since + // if a statement has no parent statement, it's either the method body + // or the body of an anonymous function declaration, in each of which cases the + // non-statement parent is in fact the enclosing callable. c.getAChildStmt+() = s } @@ -50,13 +47,10 @@ module Internal { */ cached predicate enclosingStmt(Expr e, Stmt s) { - /* - * Compute the enclosing statement for an expression. Note that this need - * not exist, since expressions can occur in contexts where they have no - * enclosing statement (examples include field initialisers, both inline - * and explicit on constructor definitions, and annotation arguments). - */ - + // Compute the enclosing statement for an expression. Note that this need + // not exist, since expressions can occur in contexts where they have no + // enclosing statement (examples include field initialisers, both inline + // and explicit on constructor definitions, and annotation arguments). getAChildExpr+(s) = e } @@ -76,14 +70,11 @@ module Internal { */ cached predicate exprEnclosingCallable(Expr e, Callable c) { - /* - * Compute the enclosing callable of an expression. Note that expressions in - * lambda functions should have the lambdas as enclosing callables, and their - * enclosing statement may be the same as the enclosing statement of the - * lambda; thus, it is *not* safe to go up to the enclosing statement and - * take its own enclosing callable. - */ - + // Compute the enclosing callable of an expression. Note that expressions in + // lambda functions should have the lambdas as enclosing callables, and their + // enclosing statement may be the same as the enclosing statement of the + // lambda; thus, it is *not* safe to go up to the enclosing statement and + // take its own enclosing callable. childExprOfCallable(c, e) or not childExprOfCallable(_, e) and diff --git a/csharp/ql/src/semmle/code/csharp/Generics.qll b/csharp/ql/src/semmle/code/csharp/Generics.qll index 782ec5b97ef..83fb04b2368 100644 --- a/csharp/ql/src/semmle/code/csharp/Generics.qll +++ b/csharp/ql/src/semmle/code/csharp/Generics.qll @@ -158,23 +158,20 @@ class TypeParameter extends DotNet::TypeParameter, Type, @type_parameter { /** Gets a type that was supplied for this parameter. */ Type getASuppliedType() { - /* - * A type parameter either comes from the source declaration - * or from a partially constructed generic. - * - * When from a source declaration, return type arguments from all ConstructedGenerics, - * and when from a partially constructed UnboundGeneric, return type arguments from - * directly ConstructedGenerics. - * - * e.g. - * - * class A { class B { } } - * - * A.B is the UnboundGenericClass source declaration, - * A.B is a partially constructed UnboundGenericClass and - * A.B is a ConstructedGenericClass. - */ - + // A type parameter either comes from the source declaration + // or from a partially constructed generic. + // + // When from a source declaration, return type arguments from all ConstructedGenerics, + // and when from a partially constructed UnboundGeneric, return type arguments from + // directly ConstructedGenerics. + // + // For example: + // + // class A { class B { } } + // + // A.B is the UnboundGenericClass source declaration, + // A.B is a partially constructed UnboundGenericClass and + // A.B is a ConstructedGenericClass. exists(ConstructedGeneric c, UnboundGeneric u, int tpi | this = u.getTypeParameter(tpi) and (u = c.getUnboundGeneric() or u = c.getSourceDeclaration()) and diff --git a/csharp/ql/src/semmle/code/csharp/Property.qll b/csharp/ql/src/semmle/code/csharp/Property.qll index adb2f44639a..6038c7c684d 100644 --- a/csharp/ql/src/semmle/code/csharp/Property.qll +++ b/csharp/ql/src/semmle/code/csharp/Property.qll @@ -178,11 +178,8 @@ class Property extends DotNet::Property, DeclarationWithGetSetAccessors, @proper override Expr getAnAssignedValue() { result = DeclarationWithGetSetAccessors.super.getAnAssignedValue() or - /* - * For library types, we don't know about assignments in constructors. We instead assume that - * arguments passed to parameters of constructors with suitable names. - */ - + // For library types, we don't know about assignments in constructors. We instead assume that + // arguments passed to parameters of constructors with suitable names. getDeclaringType().fromLibrary() and exists(Parameter param, Constructor c, string propertyName | propertyName = getName() and diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/SSA.qll b/csharp/ql/src/semmle/code/csharp/dataflow/SSA.qll index 92c1a005931..91dbdfd4cb1 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/SSA.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/SSA.qll @@ -755,15 +755,12 @@ module Ssa { BasicBlock bb, TrackedDefinition def, TrackedVar v ) { exists(BasicBlock idom | ssaDefReachesEndOfBlock(idom, def, v) | - /* - * The construction of SSA form ensures that each read of a variable is - * dominated by its definition. An SSA definition therefore reaches a - * control flow node if it is the _closest_ SSA definition that dominates - * the node. If two definitions dominate a node then one must dominate the - * other, so therefore the definition of _closest_ is given by the dominator - * tree. Thus, reaching definitions can be calculated in terms of dominance. - */ - + // The construction of SSA form ensures that each read of a variable is + // dominated by its definition. An SSA definition therefore reaches a + // control flow node if it is the _closest_ SSA definition that dominates + // the node. If two definitions dominate a node then one must dominate the + // other, so therefore the definition of _closest_ is given by the dominator + // tree. Thus, reaching definitions can be calculated in terms of dominance. idom = bb.getImmediateDominator() ) } diff --git a/csharp/ql/src/semmle/code/csharp/dispatch/Dispatch.qll b/csharp/ql/src/semmle/code/csharp/dispatch/Dispatch.qll index 9ac5d369311..f433b2706af 100644 --- a/csharp/ql/src/semmle/code/csharp/dispatch/Dispatch.qll +++ b/csharp/ql/src/semmle/code/csharp/dispatch/Dispatch.qll @@ -774,12 +774,9 @@ private module Internal { ) } - /* - * The set of static targets is all callables with matching - * names and number of parameters. This set is further reduced in - * `getADynamicTarget()` by taking type information into account. - */ - + // The set of static targets is all callables with matching + // names and number of parameters. This set is further reduced in + // `getADynamicTarget()` by taking type information into account. override Callable getAStaticTarget() { result = getACallableWithMatchingName() and exists(int minArgs | @@ -800,23 +797,20 @@ private module Internal { result.getName() = getName() } - /* - * A callable is viable if the following conditions are all satisfied: - * - * 1. It is a viable candidate (see `getADynamicTargetCandidate()`). - * 2. The argument types are compatible with the parameter types. Here, - * type compatibility means that an argument type must be implicitly - * convertible to a type that equals the corresponding parameter type - * modulo type parameters. For example, an argument type `int` is - * compatible with a parameter type `IEquatable`, because `int` is - * implicitly convertible to `IEquatable`, which equals - * `IEquatable` modulo type parameters. Note that potential type - * parameter constraints are not taken into account, nor is the - * possibility of matching a given type parameter with multiple, - * conflicting types (for example, `Tuple` is considered - * compatible with `Tuple`). - */ - + // A callable is viable if the following conditions are all satisfied: + // + // 1. It is a viable candidate (see `getADynamicTargetCandidate()`). + // 2. The argument types are compatible with the parameter types. Here, + // type compatibility means that an argument type must be implicitly + // convertible to a type that equals the corresponding parameter type + // modulo type parameters. For example, an argument type `int` is + // compatible with a parameter type `IEquatable`, because `int` is + // implicitly convertible to `IEquatable`, which equals + // `IEquatable` modulo type parameters. Note that potential type + // parameter constraints are not taken into account, nor is the + // possibility of matching a given type parameter with multiple, + // conflicting types (for example, `Tuple` is considered + // compatible with `Tuple`). override RuntimeCallable getADynamicTarget() { // Condition 1 result = getADynamicTargetCandidate() and diff --git a/csharp/ql/src/semmle/code/csharp/security/Sanitizers.qll b/csharp/ql/src/semmle/code/csharp/security/Sanitizers.qll index a6e2348d490..9d752191c64 100644 --- a/csharp/ql/src/semmle/code/csharp/security/Sanitizers.qll +++ b/csharp/ql/src/semmle/code/csharp/security/Sanitizers.qll @@ -16,16 +16,13 @@ class HtmlSanitizedExpr extends Expr { m = any(SystemWebHttpUtility c).getAnHtmlAttributeEncodeMethod() or m = any(SystemNetWebUtility h).getAnHtmlEncodeMethod() | - /* - * All four utility classes provide the same pair of Html[Attribute]Encode methods: - * - * - `string Html[Attribute]Encode(string value)` - * - `void Html[Attribute]Encode(string value, TextWriter output)` - * - * In the first form, we treat the call as sanitized, and in the second form - * we treat any subsequent uses of the `output` argument as sanitized. - */ - + // All four utility classes provide the same pair of Html[Attribute]Encode methods: + // + // - `string Html[Attribute]Encode(string value)` + // - `void Html[Attribute]Encode(string value, TextWriter output)` + // + // In the first form, we treat the call as sanitized, and in the second form + // we treat any subsequent uses of the `output` argument as sanitized. m.getNumberOfParameters() = 1 and this = m.getACall() or diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/CommandInjection.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/CommandInjection.qll index b65d6c18458..bbc5584fcdd 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/CommandInjection.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/CommandInjection.qll @@ -45,18 +45,12 @@ module CommandInjection { */ class SystemProcessCommandInjectionSink extends Sink { SystemProcessCommandInjectionSink() { - /* - * Arguments passed directly to the `System.Diagnostics.Process.Start` method. - */ - + // Arguments passed directly to the `System.Diagnostics.Process.Start` method exists(SystemDiagnosticsProcessClass processClass | this.getExpr() = processClass.getAStartMethod().getAParameter().getAnAssignedArgument() ) or - /* - * Values set on a `System.Diagnostics.ProcessStartInfo` class. - */ - + // Values set on a `System.Diagnostics.ProcessStartInfo` class exists(SystemDiagnosticsProcessStartInfoClass startInfoClass | this.getExpr() = startInfoClass.getAConstructor().getACall().getAnArgument() or diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/ConditionalBypass.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/ConditionalBypass.qll index 8510d0af833..82542dbc1c6 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/ConditionalBypass.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/ConditionalBypass.qll @@ -71,11 +71,8 @@ module UserControlledBypassOfSensitiveMethod { predicate conditionControlsMethod(SensitiveExecutionMethodCall call, Expr e) { exists(SensitiveExecutionMethod def, boolean cond | conditionControlsCall(call, def, e, cond) and - /* - * Exclude this condition if the other branch also contains a call to the same security - * sensitive method. - */ - + // Exclude this condition if the other branch also contains a call to the same security + // sensitive method. not conditionControlsCall(_, def, e, cond.booleanNot()) ) } @@ -90,12 +87,9 @@ module UserControlledBypassOfSensitiveMethod { // A condition used to guard a sensitive method call conditionControlsMethod(sensitiveMethodCall, this.getExpr()) or - /* - * A condition used to guard a sensitive method call, where the condition is `EndsWith`, - * `StartsWith` or `Contains` on a tainted value. Tracking from strings to booleans doesn't - * make sense in all contexts, so this is restricted to this case. - */ - + // A condition used to guard a sensitive method call, where the condition is `EndsWith`, + // `StartsWith` or `Contains` on a tainted value. Tracking from strings to booleans doesn't + // make sense in all contexts, so this is restricted to this case. exists(MethodCall stringComparisonCall, string methodName | methodName = "EndsWith" or methodName = "StartsWith" or diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/HardcodedCredentials.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/HardcodedCredentials.qll index 5c3f7c8ad28..b37a5104663 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/HardcodedCredentials.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/HardcodedCredentials.qll @@ -49,24 +49,18 @@ module HardcodedCredentials { override predicate isSink(DataFlow::Node sink) { sink instanceof Sink and - /* - * Ignore values that are ultimately returned by mocks, as they don't represent "real" - * credentials. - */ - + // Ignore values that are ultimately returned by mocks, as they don't represent "real" + // credentials. not any(ReturnedByMockObject mock).getAMemberInitializationValue() = sink.asExpr() and not any(ReturnedByMockObject mock).getAnArgument() = sink.asExpr() } override predicate hasFlow(DataFlow::Node source, DataFlow::Node sink) { super.hasFlow(source, sink) and - /* - * Exclude hard-coded credentials in tests if they only flow to calls to methods with a name - * like "Add*" "Create*" or "Update*". The rationale is that hard-coded credentials within - * tests that are only used for creating or setting values within tests are unlikely to - * represent credentials to some accessible system. - */ - + // Exclude hard-coded credentials in tests if they only flow to calls to methods with a name + // like "Add*" "Create*" or "Update*". The rationale is that hard-coded credentials within + // tests that are only used for creating or setting values within tests are unlikely to + // represent credentials to some accessible system. not ( source.asExpr().getFile() instanceof TestFile and exists(MethodCall createOrAddCall, string createOrAddMethodName | diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/MissingXMLValidation.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/MissingXMLValidation.qll index c6e631089da..06638a0c762 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/MissingXMLValidation.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/MissingXMLValidation.qll @@ -64,16 +64,13 @@ module MissingXMLValidation { result = "there is no 'XmlReaderSettings' instance specifying schema validation." and not exists(createCall.getSettings()) or - /* - * An XmlReaderSettings instance is passed where: - * - The ValidationType is not set to Schema; or - * - The ValidationType is set to Schema, but: - * - The ProcessInlineSchema option is set (this allows the document to set a schema - * internally); or - * - The ProcessSchemaLocation option is set (this allows the document to reference a - * schema by location that this document will validate against). - */ - + // An XmlReaderSettings instance is passed where: + // - The ValidationType is not set to Schema; or + // - The ValidationType is set to Schema, but: + // - The ProcessInlineSchema option is set (this allows the document to set a schema + // internally); or + // - The ProcessSchemaLocation option is set (this allows the document to reference a + // schema by location that this document will validate against). result = "the 'XmlReaderSettings' instance does not specify the 'ValidationType' as 'Schema'." and exists(XmlReaderSettingsCreation settingsCreation | settingsCreation = createCall.getSettings().getASettingsCreation() diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/TaintedPath.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/TaintedPath.qll index 6ce50c33789..bb1832c39ac 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/TaintedPath.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/TaintedPath.qll @@ -123,10 +123,7 @@ module TaintedPath { */ class PathCheck extends Sanitizer { PathCheck() { - /* - * This expression is structurally replicated in a dominating guard which is not a "weak" check. - */ - + // This expression is structurally replicated in a dominating guard which is not a "weak" check. exists(Expr e | this.getExpr().(GuardedExpr).isGuardedBy(_, e, _) and not inWeakCheck(e) diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll index b8d52faaeb3..ae6a9980cf2 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/XSS.qll @@ -184,12 +184,9 @@ module XSS { m = writeClass.getAWriteAttributeMethod() ) or - /* - * The second parameter to the `WriteAttribute` method is the attribute value, which we - * should only consider as tainted if the call does not ask for the attribute value to be - * encoded using the final parameter. - */ - + // The second parameter to the `WriteAttribute` method is the attribute value, which we + // should only consider as tainted if the call does not ask for the attribute value to be + // encoded using the final parameter. m = writeClass.getAWriteAttributeMethod() and paramPos = 1 and not c.getArgumentForParameter(m.getParameter(2)).(BoolLiteral).getBoolValue() = true @@ -378,13 +375,10 @@ module XSS { * `f1.f2...fn[...]()`. The `i`th member is `fi` in all cases. */ private string getMemberAccessNameByIndex(AspInlineCode code, int i) { - /* - * Strip: - * - leading and trailing whitespace, which apparently you're allowed to have - * - trailing parens, so we can recognize nullary method calls - * - trailing square brackets with some contents, to recognize indexing into arrays - */ - + // Strip: + // - leading and trailing whitespace, which apparently you're allowed to have + // - trailing parens, so we can recognize nullary method calls + // - trailing square brackets with some contents, to recognize indexing into arrays result = code.getBody().splitAt(".", i).regexpCapture("\\s*(.*?)(\\[.*\\])?(\\(\\))?\\s*", 1) } diff --git a/csharp/ql/src/semmle/code/csharp/security/dataflow/ZipSlip.qll b/csharp/ql/src/semmle/code/csharp/security/dataflow/ZipSlip.qll index b7032d2599f..e5787a313f9 100644 --- a/csharp/ql/src/semmle/code/csharp/security/dataflow/ZipSlip.qll +++ b/csharp/ql/src/semmle/code/csharp/security/dataflow/ZipSlip.qll @@ -133,11 +133,8 @@ module ZipSlip { | mc.getTarget().hasQualifiedName("System.String", "StartsWith") and mc.getQualifier() = startsWithQualifier and - /* - * A StartsWith check against Path.Combine is not sufficient, because the ".." elements have - * not yet been resolved. - */ - + // A StartsWith check against Path.Combine is not sufficient, because the ".." elements have + // not yet been resolved. not exists(MethodCall combineCall | combineCall.getTarget().hasQualifiedName("System.IO.Path", "Combine") and DataFlow::localFlow(DataFlow::exprNode(combineCall), diff --git a/csharp/ql/src/semmle/code/csharp/security/xml/InsecureXML.qll b/csharp/ql/src/semmle/code/csharp/security/xml/InsecureXML.qll index d279d7ef74d..8ddd396219c 100644 --- a/csharp/ql/src/semmle/code/csharp/security/xml/InsecureXML.qll +++ b/csharp/ql/src/semmle/code/csharp/security/xml/InsecureXML.qll @@ -13,12 +13,9 @@ module InsecureXML { */ bindingset[version] private predicate isNetFrameworkBefore(Type t, string version) { - /* - * For assemblies compiled against framework versions before 4 the TargetFrameworkAttribute - * will not be present. In this case, we can revert back to the assembly version, which may not - * contain full minor version information. - */ - + // For assemblies compiled against framework versions before 4 the TargetFrameworkAttribute + // will not be present. In this case, we can revert back to the assembly version, which may not + // contain full minor version information. exists(string assemblyVersion | assemblyVersion = t .getALocation() @@ -31,11 +28,8 @@ module InsecureXML { assemblyVersion.toFloat() < 4.0 ) or - /* - * For 4.0 and above the TargetFrameworkAttribute should be present to provide detailed version - * information. - */ - + // For 4.0 and above the TargetFrameworkAttribute should be present to provide detailed version + // information. exists(TargetFrameworkAttribute tfa | tfa.hasElement(t) and tfa.isNetFramework() and diff --git a/csharp/ql/src/semmle/code/dotnet/Expr.qll b/csharp/ql/src/semmle/code/dotnet/Expr.qll index 70030d61989..29a9c6c170d 100644 --- a/csharp/ql/src/semmle/code/dotnet/Expr.qll +++ b/csharp/ql/src/semmle/code/dotnet/Expr.qll @@ -29,7 +29,7 @@ class Call extends Expr, @dotnet_call { /** Gets the target of this call. */ Callable getTarget() { none() } - /* Gets any potential target of this call. */ + /** Gets any potential target of this call. */ Callable getARuntimeTarget() { none() } /** diff --git a/csharp/ql/test/library-tests/types/Types36.ql b/csharp/ql/test/library-tests/types/Types36.ql index 80535ddf54f..cf9c897996b 100644 --- a/csharp/ql/test/library-tests/types/Types36.ql +++ b/csharp/ql/test/library-tests/types/Types36.ql @@ -1,4 +1,3 @@ -/* Ensure that built-in types can be interfaces */ import csharp from Interface i