Skip to content

Commit

Permalink
refacto update isrequire/isunsafecallee, remove new probe
Browse files Browse the repository at this point in the history
  • Loading branch information
tchapacan committed Dec 22, 2023
1 parent b624041 commit 3fb5201
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 89 deletions.
4 changes: 1 addition & 3 deletions src/probes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,15 @@ import isUnaryExpression from "./isUnaryExpression.js";
import isWeakCrypto from "./isWeakCrypto.js";
import isClassDeclaration from "./isClassDeclaration.js";
import isMethodDefinition from "./isMethodDefinition.js";
import isUnsafeEvalRequire from "./isUnsafeEvalRequire.js";

// CONSTANTS
const kListOfProbes = [
isUnsafeEvalRequire,
isRequire,
isUnsafeCallee,
isLiteral,
isLiteralRegex,
isRegexObject,
isVariableDeclaration,
isRequire,
isImportDeclaration,
isMemberExpression,
isAssignmentExpression,
Expand Down
40 changes: 32 additions & 8 deletions src/probes/isRequire.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,50 @@ import {
getCallExpressionArguments
} from "@nodesecure/estree-ast-utils";

function validateNode(node, { tracer }) {
function validateNode(node, option) {
const id = getCallExpressionIdentifier(node);
const argument = getCallExpressionArguments(node);

if (id === null) {
return [false];
}

const data = tracer.getDataFromIdentifier(id);
const data = option.tracer.getDataFromIdentifier(id);

if (data !== null && data.name === "require") {
return [
true,
data?.identifierOrMemberExpr ?? void 0
];
}
else if (id === "eval" && argument[0] === "require") {
return [
true,
option?.identifiersName[0].name ?? void 0
];
}

return [
data !== null && data.name === "require",
data?.identifierOrMemberExpr ?? void 0
false,
null
];
}

function main(node, options) {
const { analysis } = options;
const { analysis, data: calleeName } = options;
const { tracer } = analysis;

if (node.arguments.length === 0) {
return;
}
const arg = node.arguments.at(0);
const id = getCallExpressionIdentifier(node);


if (arg.value === "require" && id === "eval") {
analysis.addWarning("unsafe-import", calleeName, node.loc);
analysis.addDependency(calleeName, node.loc, true);
}

switch (arg.type) {
// const foo = "http"; require(foo);
Expand All @@ -48,12 +70,14 @@ function main(node, options) {
}
break;

// require("http")
// require("http")
case "Literal":
analysis.addDependency(arg.value, node.loc);
if (arg.value !== "require") {
analysis.addDependency(arg.value, node.loc);
}
break;

// require(["ht", "tp"])
// require(["ht", "tp"])
case "ArrayExpression": {
const value = [...arrayExpressionToString(arg, { tracer })]
.join("")
Expand Down
2 changes: 0 additions & 2 deletions src/probes/isUnsafeCallee.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ function main(node, options) {
const { analysis, data: calleeName } = options;

analysis.addWarning("unsafe-stmt", calleeName, node.loc);

return Symbol.for("skipWalk");
}

export default {
Expand Down
40 changes: 0 additions & 40 deletions src/probes/isUnsafeEvalRequire.js

This file was deleted.

3 changes: 2 additions & 1 deletion src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ export function isUnsafeCallee(node) {
// For Function we are looking for this: `Function("...")();`
// A double CallExpression
return [
identifier === "eval" || (identifier === "Function" && node.callee.type === "CallExpression"),
(identifier === "eval" && node.callee.type === "Identifier") ||
(identifier === "Function" && node.callee.type === "CallExpression"),
identifier
];
}
Expand Down
3 changes: 3 additions & 0 deletions test/issues/179-UnsafeEvalRequire.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,7 @@ test("should detect unsafe-import and unsafe-statement", () => {
assert.equal(sastAnalysis.warnings.at(1).value, "eval");
assert.equal(sastAnalysis.warnings.at(1).kind, kWarningUnsafeStatement);
assert.equal(sastAnalysis.warnings.length, 2);
assert.equal(sastAnalysis.dependencies.has("stream"), true);
assert.equal(sastAnalysis.dependencies.get("stream").unsafe, true);
assert.equal(sastAnalysis.dependencies.size, 1);
});
35 changes: 0 additions & 35 deletions test/probes/isUnsafeEvalRequire.spec.js

This file was deleted.

0 comments on commit 3fb5201

Please sign in to comment.