Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify ReturnStatement to prevent a UaF #76

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 35 additions & 5 deletions src/nodes/statements.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as ts from 'typescript';
import { CodeTemplate, CodeTemplateFactory, CTemplateBase } from '../template';
import { CProgram, IScope} from '../program';
import { CProgram, IScope } from '../program';
import { ArrayType, NumberVarType, StringVarType } from '../types/ctypes';
import { CVariable, CVariableDeclaration, CVariableDestructors } from './variable';
import { CExpression, CCondition } from './expressions';
Expand Down Expand Up @@ -71,17 +71,48 @@ export class CEmptyStatement {
}

@CodeTemplate(`
{destructors}
return {expression};
{#if returnTypeAndVar}
{
{returnTypeAndVar} = {expression};
{destructors}
return {returnTemp};
}
{#else}
{destructors}
return {expression};
{/if}
`, ts.SyntaxKind.ReturnStatement)
export class CReturnStatement extends CTemplateBase {
public expression: CExpression;
public destructors: CVariableDestructors;
public retVarName: string = null;
public returnTypeAndVar: string = null;
public returnTemp: string = null;
public closureParams: { name: string, value: CExpression }[] = [];
doCheckVarNeeded(scope: IScope, node: ts.ReturnStatement): boolean {
let s = scope.root.memoryManager.getDestructorsForScope(node);
let result = false;
for (let e of s) {
result = result || new RegExp('\\W' + e.varName + '\\W').test(' ' + node.getFullText() + ' ');
}
return result;
}
constructor(scope: IScope, node: ts.ReturnStatement) {
super();
this.returnTemp = scope.root.symbolsHelper.addTemp(node, 'returnVal');
let returnType = scope.root.typeHelper.getCType(node.expression);
if (this.doCheckVarNeeded(scope, node)) {
// todo: make this less hackyy
let fakeVar = new CVariable(scope, '__fake', returnType, { removeStorageSpecifier: true, arraysToPointers: true });;
let fakeVarType = fakeVar.resolve().slice(0, -6).trim();
this.returnTypeAndVar = fakeVarType;
if (this.returnTypeAndVar.indexOf('{var}') == -1) {
this.returnTypeAndVar += ' {var}';
}
this.returnTypeAndVar = this.returnTypeAndVar.replace('{var}', this.returnTemp);
}
this.expression = CodeTemplateFactory.createForNode(scope, node.expression);

this.destructors = new CVariableDestructors(scope, node);
}
}
Expand Down Expand Up @@ -380,8 +411,7 @@ export class CForInStatement extends CTemplateBase implements IScope {
else
this.init = new CElementAccess(scope, node.initializer);

if (node.statement.kind == ts.SyntaxKind.Block)
{
if (node.statement.kind == ts.SyntaxKind.Block) {
let block = <ts.Block>node.statement;
for (let s of block.statements)
this.statements.push(CodeTemplateFactory.createForNode(this, s));
Expand Down
76 changes: 37 additions & 39 deletions src/nodes/variable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,45 +113,43 @@ export class CVariableAllocation extends CTemplateBase {
}

@CodeTemplate(`
{#statements}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think {#statements} was there for a reason...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know, but i know the tests pass... I removed statements because it moved it above the actual return code.

{arrayDestructors => for (gc_i = 0; gc_i < ({this} ? {this}->size : 0); gc_i++) free((void*){this}->data[gc_i]);\n}
{destructors => free({this});\n}
{#if gcArraysCVarName}
for (gc_i = 0; gc_i < {gcArraysCVarName}->size; gc_i++) {
for (gc_j = 0; gc_j < ({gcArraysCVarName}->data[gc_i] ? {gcArraysCVarName}->data[gc_i]->size : 0); gc_j++)
free((void*){gcArraysCVarName}->data[gc_i]->data[gc_j]);\n
free({gcArraysCVarName}->data[gc_i] ? {gcArraysCVarName}->data[gc_i]->data : NULL);
free({gcArraysCVarName}->data[gc_i]);
}
free({gcArraysCVarName}->data);
free({gcArraysCVarName});
{/if}
{#if gcArraysVarName}
for (gc_i = 0; gc_i < {gcArraysVarName}->size; gc_i++) {
free({gcArraysVarName}->data[gc_i]->data);
free({gcArraysVarName}->data[gc_i]);
}
free({gcArraysVarName}->data);
free({gcArraysVarName});
{/if}
{#if gcDictsVarName}
for (gc_i = 0; gc_i < {gcDictsVarName}->size; gc_i++) {
free({gcDictsVarName}->data[gc_i]->index->data);
free({gcDictsVarName}->data[gc_i]->index);
free({gcDictsVarName}->data[gc_i]->values->data);
free({gcDictsVarName}->data[gc_i]->values);
free({gcDictsVarName}->data[gc_i]);
}
free({gcDictsVarName}->data);
free({gcDictsVarName});
{/if}
{#if gcVarName}
for (gc_i = 0; gc_i < {gcVarName}->size; gc_i++)
free({gcVarName}->data[gc_i]);
free({gcVarName}->data);
free({gcVarName});
{/if}
{/statements}`
{arrayDestructors => for (gc_i = 0; gc_i < ({this} ? {this}->size : 0); gc_i++) free((void*){this}->data[gc_i]);\n}
{destructors => free({this});\n}
{#if gcArraysCVarName}
for (gc_i = 0; gc_i < {gcArraysCVarName}->size; gc_i++) {
for (gc_j = 0; gc_j < ({gcArraysCVarName}->data[gc_i] ? {gcArraysCVarName}->data[gc_i]->size : 0); gc_j++)
free((void*){gcArraysCVarName}->data[gc_i]->data[gc_j]);\n
free({gcArraysCVarName}->data[gc_i] ? {gcArraysCVarName}->data[gc_i]->data : NULL);
free({gcArraysCVarName}->data[gc_i]);
}
free({gcArraysCVarName}->data);
free({gcArraysCVarName});
{/if}
{#if gcArraysVarName}
for (gc_i = 0; gc_i < {gcArraysVarName}->size; gc_i++) {
free({gcArraysVarName}->data[gc_i]->data);
free({gcArraysVarName}->data[gc_i]);
}
free({gcArraysVarName}->data);
free({gcArraysVarName});
{/if}
{#if gcDictsVarName}
for (gc_i = 0; gc_i < {gcDictsVarName}->size; gc_i++) {
free({gcDictsVarName}->data[gc_i]->index->data);
free({gcDictsVarName}->data[gc_i]->index);
free({gcDictsVarName}->data[gc_i]->values->data);
free({gcDictsVarName}->data[gc_i]->values);
free({gcDictsVarName}->data[gc_i]);
}
free({gcDictsVarName}->data);
free({gcDictsVarName});
{/if}
{#if gcVarName}
for (gc_i = 0; gc_i < {gcVarName}->size; gc_i++)
free({gcVarName}->data[gc_i]);
free({gcVarName}->data);
free({gcVarName});
{/if}`
)
export class CVariableDestructors extends CTemplateBase {
public gcVarName: string = null;
Expand Down
14 changes: 8 additions & 6 deletions src/template.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {IScope} from './program';
import { IScope } from './program';

interface INode { kind: number, getText(): string };

Expand All @@ -16,7 +16,7 @@ export class CodeTemplateFactory {
: "/* Unsupported node: " + node.getText().replace(/[\n\s]+/g, ' ') + " */;\n";
}
public static templateToString(template: string | CTemplateBase) {
return typeof(template) === "string" ? template : template.resolve();
return typeof (template) === "string" ? template : template.resolve();
}
}

Expand All @@ -42,6 +42,8 @@ export function CodeTemplate(tempString: string, nodeKind?: number | number[]):
nodeKindTemplates[nk] = newConstructor;
}

newConstructor.prototype = target.prototype;

return newConstructor;

} as ClassDecorator;
Expand Down Expand Up @@ -123,15 +125,15 @@ function processTemplate(template: string, args: string | CTemplateBase): [strin
let index = -1;
while ((index = template.indexOf("{" + k + "}")) > -1) {
let spaces = '';
while (template.length > index && template[index-1] == ' ') {
while (template.length > index && template[index - 1] == ' ') {
index--;
spaces+=' ';
spaces += ' ';
}
let value = args[k];
if (value && value.resolve)
value = value.resolve();
if (value && typeof value === 'string')
value = value.replace(/\n/g, '\n'+spaces);
value = value.replace(/\n/g, '\n' + spaces);
template = template.replace("{" + k + "}", () => value);
replaced = true;
}
Expand Down Expand Up @@ -226,7 +228,7 @@ function replaceArray(data, k, array, statements) {
if (elementsResolved != "")
elementsResolved += separator;
elementsResolved += resolvedElement;
}
}

}

Expand Down
1 change: 1 addition & 0 deletions tests/regression/regression1.res.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1
7 changes: 7 additions & 0 deletions tests/regression/regression1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// regression test for #74
function return_from_obj() {
let obj = { key: 1 };
return obj.key;
}

console.log(return_from_obj());