Skip to content

Commit 1a1b7d4

Browse files
asgerfasgerf
committed
JS: Switch to whitelisting allowed properties
1 parent 6645df9 commit 1a1b7d4

File tree

4 files changed

+231
-55
lines changed

4 files changed

+231
-55
lines changed

‎javascript/extractor/lib/typescript/src/main.ts

Lines changed: 139 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,11 @@ interface PrepareFilesCommand {
6969
command: "prepare-files";
7070
filenames: string[];
7171
}
72+
interface GetMetadataCommand {
73+
command: "get-metadata";
74+
}
7275
type Command = ParseCommand | OpenProjectCommand | CloseProjectCommand
73-
| GetTypeTableCommand | ResetCommand | QuitCommand | PrepareFilesCommand;
76+
| GetTypeTableCommand | ResetCommand | QuitCommand | PrepareFilesCommand | GetMetadataCommand;
7477

7578
/** The state to be shared between commands. */
7679
class State {
@@ -91,7 +94,7 @@ const reloadMemoryThresholdMb = getEnvironmentVariable("SEMMLE_TYPESCRIPT_MEMORY
9194
/**
9295
* Debugging method for finding cycles in the TypeScript AST. Should not be used in production.
9396
*
94-
* If cycles are found, additional properties should be added to `isBlacklistedProperty`.
97+
* If cycles are found, the whitelist in `astProperties` is too permissive.
9598
*/
9699
// tslint:disable-next-line:no-unused-variable
97100
function checkCycle(root: any) {
@@ -104,7 +107,7 @@ function checkCycle(root: any) {
104107
obj.$cycle_visiting = true;
105108
for (let k in obj) {
106109
if (!obj.hasOwnProperty(k)) continue;
107-
if (isBlacklistedProperty(k)) continue;
110+
if (+k !== +k && !astPropertySet.has(k)) continue;
108111
if (k === "$cycle_visiting") continue;
109112
let cycle = visit(obj[k]);
110113
if (cycle) {
@@ -122,31 +125,133 @@ function checkCycle(root: any) {
122125
}
123126
}
124127

125-
/**
126-
* A property that should not be serialized as part of the AST, because they
127-
* lead to cycles or are just not needed.
128-
*
129-
* Because of restrictions on `JSON.stringify`, these properties may also not
130-
* be used as part of a command response.
131-
*/
132-
function isBlacklistedProperty(k: string) {
133-
return k === "parent" || k === "pos" || k === "end"
134-
|| k === "symbol" || k === "localSymbol"
135-
|| k === "flowNode" || k === "returnFlowNode" || k === "endFlowNode" || k === "fallthroughFlowNode"
136-
|| k === "nextContainer" || k === "locals"
137-
|| k === "bindDiagnostics" || k === "bindSuggestionDiagnostics"
138-
|| k === "relatedInformation";
139-
}
128+
/** Property names to extract from the TypeScript AST. */
129+
const astProperties: string[] = [
130+
"$declarationKind",
131+
"$declaredSignature",
132+
"$end",
133+
"$lineStarts",
134+
"$overloadIndex",
135+
"$pos",
136+
"$resolvedSignature",
137+
"$symbol",
138+
"$tokens",
139+
"$type",
140+
"argument",
141+
"argumentExpression",
142+
"arguments",
143+
"assertsModifier",
144+
"asteriskToken",
145+
"attributes",
146+
"block",
147+
"body",
148+
"caseBlock",
149+
"catchClause",
150+
"checkType",
151+
"children",
152+
"clauses",
153+
"closingElement",
154+
"closingFragment",
155+
"condition",
156+
"constraint",
157+
"constructor",
158+
"declarationList",
159+
"declarations",
160+
"decorators",
161+
"default",
162+
"delete",
163+
"dotDotDotToken",
164+
"elements",
165+
"elementType",
166+
"elementTypes",
167+
"elseStatement",
168+
"escapedText",
169+
"exclamationToken",
170+
"exportClause",
171+
"expression",
172+
"exprName",
173+
"extendsType",
174+
"falseType",
175+
"finallyBlock",
176+
"flags",
177+
"head",
178+
"heritageClauses",
179+
"importClause",
180+
"incrementor",
181+
"indexType",
182+
"init",
183+
"initializer",
184+
"isExportEquals",
185+
"isTypeOf",
186+
"isTypeOnly",
187+
"keywordToken",
188+
"kind",
189+
"label",
190+
"left",
191+
"literal",
192+
"members",
193+
"messageText",
194+
"modifiers",
195+
"moduleReference",
196+
"moduleSpecifier",
197+
"name",
198+
"namedBindings",
199+
"objectType",
200+
"openingElement",
201+
"openingFragment",
202+
"operand",
203+
"operator",
204+
"operatorToken",
205+
"parameterName",
206+
"parameters",
207+
"parseDiagnostics",
208+
"properties",
209+
"propertyName",
210+
"qualifier",
211+
"questionDotToken",
212+
"questionToken",
213+
"right",
214+
"selfClosing",
215+
"statement",
216+
"statements",
217+
"tag",
218+
"tagName",
219+
"template",
220+
"templateSpans",
221+
"text",
222+
"thenStatement",
223+
"token",
224+
"tokenPos",
225+
"trueType",
226+
"tryBlock",
227+
"type",
228+
"typeArguments",
229+
"typeName",
230+
"typeParameter",
231+
"typeParameters",
232+
"types",
233+
"variableDeclaration",
234+
"whenFalse",
235+
"whenTrue",
236+
];
237+
238+
/** Property names used in a parse command response, in addition to the AST itself. */
239+
const astMetaProperties: string[] = [
240+
"ast",
241+
"type",
242+
];
243+
244+
/** Property names to extract in an AST response. */
245+
const astPropertySet = new Set([...astProperties, ...astMetaProperties]);
140246

141247
/**
142-
* Converts (part of) an AST to a JSON string, ignoring parent pointers.
248+
* Converts (part of) an AST to a JSON string, ignoring properties we're not interested in.
143249
*/
144250
function stringifyAST(obj: any) {
145251
return JSON.stringify(obj, (k, v) => {
146-
if (isBlacklistedProperty(k)) {
147-
return undefined;
148-
}
149-
return v;
252+
// Filter out properties that aren't numeric, empty, or whitelisted.
253+
// Note `k` is the empty string for the root object, which is also covered by +k === +k.
254+
return (+k === +k || astPropertySet.has(k)) ? v : undefined;
150255
});
151256
}
152257

@@ -156,8 +261,6 @@ function extractFile(filename: string): string {
156261
return stringifyAST({
157262
type: "ast",
158263
ast,
159-
nodeFlags: ts.NodeFlags,
160-
syntaxKinds: ts.SyntaxKind
161264
});
162265
}
163266

@@ -523,6 +626,14 @@ function handlePrepareFilesCommand(command: PrepareFilesCommand) {
523626
});
524627
}
525628

629+
function handleGetMetadataCommand(command: GetMetadataCommand) {
630+
console.log(JSON.stringify({
631+
type: "metadata",
632+
syntaxKinds: ts.SyntaxKind,
633+
nodeFlags: ts.NodeFlags,
634+
}));
635+
}
636+
526637
function reset() {
527638
state = new State();
528639
state.typeTable.restrictedExpansion = getEnvironmentVariable("SEMMLE_TYPESCRIPT_NO_EXPANSION", Boolean, true);
@@ -583,6 +694,9 @@ function runReadLineInterface() {
583694
case "reset":
584695
handleResetCommand(req);
585696
break;
697+
case "get-metadata":
698+
handleGetMetadataCommand(req);
699+
break;
586700
case "quit":
587701
rl.close();
588702
break;

‎javascript/extractor/src/com/semmle/js/parser/TypeScriptASTConverter.java

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@
22

33
import java.util.ArrayList;
44
import java.util.Collections;
5-
import java.util.LinkedHashMap;
65
import java.util.List;
7-
import java.util.Map;
86
import java.util.regex.Matcher;
97
import java.util.regex.Pattern;
108

@@ -163,10 +161,7 @@
163161
*/
164162
public class TypeScriptASTConverter {
165163
private String source;
166-
private final JsonObject nodeFlags;
167-
private final JsonObject syntaxKinds;
168-
private final Map<Integer, String> nodeFlagMap = new LinkedHashMap<>();
169-
private final Map<Integer, String> syntaxKindMap = new LinkedHashMap<>();
164+
private final TypeScriptParserMetadata metadata;
170165
private int[] lineStarts;
171166

172167
private int syntaxKindExtends;
@@ -180,24 +175,11 @@ public class TypeScriptASTConverter {
180175
private static final Pattern WHITESPACE_END_PAREN =
181176
Pattern.compile("^" + WHITESPACE_CHAR + "*\\)");
182177

183-
TypeScriptASTConverter(JsonObject nodeFlags, JsonObject syntaxKinds) {
184-
this.nodeFlags = nodeFlags;
185-
this.syntaxKinds = syntaxKinds;
186-
makeEnumIdMap(nodeFlags, nodeFlagMap);
187-
makeEnumIdMap(syntaxKinds, syntaxKindMap);
178+
TypeScriptASTConverter(TypeScriptParserMetadata metadata) {
179+
this.metadata = metadata;
188180
this.syntaxKindExtends = getSyntaxKind("ExtendsKeyword");
189181
}
190182

191-
/** Builds a mapping from ID to name given a TypeScript enum object. */
192-
private void makeEnumIdMap(JsonObject enumObject, Map<Integer, String> idToName) {
193-
for (Map.Entry<String, JsonElement> entry : enumObject.entrySet()) {
194-
JsonPrimitive prim = entry.getValue().getAsJsonPrimitive();
195-
if (prim.isNumber() && !idToName.containsKey(prim.getAsInt())) {
196-
idToName.put(prim.getAsInt(), entry.getKey());
197-
}
198-
}
199-
}
200-
201183
/**
202184
* Convert the given TypeScript AST (which was parsed from {@code source}) into a parser {@link
203185
* Result}.
@@ -1617,7 +1599,7 @@ private Node convertMappedType(JsonObject node, SourceLocation loc) throws Parse
16171599
private Node convertMetaProperty(JsonObject node, SourceLocation loc) throws ParseError {
16181600
Position metaStart = loc.getStart();
16191601
String keywordKind =
1620-
syntaxKinds.get(node.getAsJsonPrimitive("keywordToken").getAsInt() + "").getAsString();
1602+
metadata.getSyntaxKinds().get(node.getAsJsonPrimitive("keywordToken").getAsInt() + "").getAsString();
16211603
String identifier = keywordKind.equals("ImportKeyword") ? "import" : "new";
16221604
Position metaEnd =
16231605
new Position(
@@ -1995,7 +1977,7 @@ private Node convertPrefixUnaryExpression(JsonObject node, SourceLocation loc) t
19951977

19961978
private String getOperator(JsonObject node) throws ParseError {
19971979
int operatorId = node.get("operator").getAsInt();
1998-
switch (syntaxKindMap.get(operatorId)) {
1980+
switch (metadata.getSyntaxKindMap().get(operatorId)) {
19991981
case "PlusPlusToken":
20001982
return "++";
20011983
case "MinusMinusToken":
@@ -2219,7 +2201,7 @@ private Node convertTypeOfExpression(JsonObject node, SourceLocation loc) throws
22192201
}
22202202

22212203
private Node convertTypeOperator(JsonObject node, SourceLocation loc) throws ParseError {
2222-
String operator = syntaxKinds.get("" + node.get("operator").getAsInt()).getAsString();
2204+
String operator = metadata.getSyntaxKinds().get("" + node.get("operator").getAsInt()).getAsString();
22232205
if (operator.equals("KeyOfKeyword")) {
22242206
return new UnaryTypeExpr(loc, UnaryTypeExpr.Kind.Keyof, convertChildAsType(node, "type"));
22252207
}
@@ -2537,7 +2519,7 @@ private int getMemberModifierKeywords(JsonObject node) {
25372519
* <tt>ts.NodeFlags</tt> in enum.
25382520
*/
25392521
private boolean hasFlag(JsonObject node, String flagName) {
2540-
JsonElement flagDescriptor = this.nodeFlags.get(flagName);
2522+
JsonElement flagDescriptor = this.metadata.getNodeFlags().get(flagName);
25412523
if (flagDescriptor == null) {
25422524
throw new RuntimeException(
25432525
"Incompatible version of TypeScript installed. Missing node flag " + flagName);
@@ -2552,7 +2534,7 @@ private boolean hasFlag(JsonObject node, String flagName) {
25522534

25532535
/** Gets the numeric value of the syntax kind enum with the given name. */
25542536
private int getSyntaxKind(String syntaxKind) {
2555-
JsonElement descriptor = this.syntaxKinds.get(syntaxKind);
2537+
JsonElement descriptor = this.metadata.getSyntaxKinds().get(syntaxKind);
25562538
if (descriptor == null) {
25572539
throw new RuntimeException(
25582540
"Incompatible version of TypeScript installed. Missing syntax kind " + syntaxKind);
@@ -2581,7 +2563,7 @@ private String getKind(JsonElement node) {
25812563
if (node instanceof JsonObject) {
25822564
JsonElement kind = ((JsonObject) node).get("kind");
25832565
if (kind instanceof JsonPrimitive && ((JsonPrimitive) kind).isNumber())
2584-
return syntaxKindMap.get(kind.getAsInt());
2566+
return metadata.getSyntaxKindMap().get(kind.getAsInt());
25852567
}
25862568
return null;
25872569
}

‎javascript/extractor/src/com/semmle/js/parser/TypeScriptParser.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,9 @@ public class TypeScriptParser {
150150
/** If non-zero, we use this instead of relying on the corresponding environment variable. */
151151
private int typescriptRam = 0;
152152

153+
/** Metadata requested immediately after starting the TypeScript parser. */
154+
private TypeScriptParserMetadata metadata;
155+
153156
/** Sets the amount of RAM to allocate to the TypeScript compiler.s */
154157
public void setTypescriptRam(int megabytes) {
155158
this.typescriptRam = megabytes;
@@ -297,6 +300,7 @@ private void setupParserWrapper() {
297300
InputStream is = parserWrapperProcess.getInputStream();
298301
InputStreamReader isr = new InputStreamReader(is, "UTF-8");
299302
fromParserWrapper = new BufferedReader(isr);
303+
this.loadMetadata();
300304
} catch (IOException e) {
301305
throw new CatastrophicError(
302306
"Could not start TypeScript parser wrapper " + "(command: ." + parserWrapperCommand + ")",
@@ -385,6 +389,17 @@ private RuntimeException getExceptionFromMalformedResponse(String response, Exce
385389
return new CatastrophicError("Unexpected response from TypeScript parser wrapper:\n" + response, e);
386390
}
387391

392+
/**
393+
* Requests metadata from the TypeScript process. See {@link TypeScriptParserMetadata}.
394+
*/
395+
private void loadMetadata() {
396+
JsonObject request = new JsonObject();
397+
request.add("command", new JsonPrimitive("get-metadata"));
398+
JsonObject response = talkToParserWrapper(request);
399+
checkResponseType(response, "metadata");
400+
this.metadata = new TypeScriptParserMetadata(response);
401+
}
402+
388403
/**
389404
* Returns the AST for a given source file.
390405
*
@@ -402,11 +417,9 @@ public Result parse(File sourceFile, String source, ExtractionMetrics metrics) {
402417
metrics.stopPhase(ExtractionMetrics.ExtractionPhase.TypeScriptParser_talkToParserWrapper);
403418
try {
404419
checkResponseType(response, "ast");
405-
JsonObject nodeFlags = response.get("nodeFlags").getAsJsonObject();
406-
JsonObject syntaxKinds = response.get("syntaxKinds").getAsJsonObject();
407420
JsonObject ast = response.get("ast").getAsJsonObject();
408421
metrics.startPhase(ExtractionMetrics.ExtractionPhase.TypeScriptASTConverter_convertAST);
409-
Result converted = new TypeScriptASTConverter(nodeFlags, syntaxKinds).convertAST(ast, source);
422+
Result converted = new TypeScriptASTConverter(metadata).convertAST(ast, source);
410423
metrics.stopPhase(ExtractionMetrics.ExtractionPhase.TypeScriptASTConverter_convertAST);
411424
return converted;
412425
} catch (IllegalStateException e) {

0 commit comments

Comments
 (0)