Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Commit

Permalink
Merge pull request #10 from webschik/tsr-detect-sql-literal-injection
Browse files Browse the repository at this point in the history
Added tsr-detect-sql-literal-injection rule
  • Loading branch information
webschik authored Oct 13, 2018
2 parents 8f4ce14 + f00c7c3 commit e58c93f
Show file tree
Hide file tree
Showing 8 changed files with 156 additions and 7 deletions.
28 changes: 26 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Inspired by [eslint-plugin-security](https://github.com/nodesecurity/eslint-plug

## How to use
* Install package:
```
```shell
npm i tslint-config-security --save-dev --production
```

Expand All @@ -22,6 +22,17 @@ npm i tslint-config-security --save-dev --production
}
```

By default `tslint-config-security` enables [all rules](#rules), but you may disable any of them (not recommended):

```json
{
"extends": ["tslint-config-security"],
"rules": {
"tsr-detect-html-injection": false,
"tsr-detect-unsafe-regexp": false
}
```


## Rules
All rules start from the prefix `tsr-` (TSLint Security Rule) to prevent name collisions.
Expand Down Expand Up @@ -82,7 +93,7 @@ Due to the known issues in the typed TSLint rules

`tslint-config-security` module will analyze methods only on **fs** variable or on **'fs' module**. E.g.:

```
```js
const fs = require('fs');

fs.open(somePath); // triggers the error
Expand Down Expand Up @@ -126,3 +137,16 @@ More information: http://stackoverflow.com/questions/18130254/randombytes-vs-pse
Detects HTML injections:
- `document.write(variable)`
- `Element.innerHTML = variable;`

#### `tsr-detect-sql-literal-injection`

Detects possible SQL injections in string literals:
```js
const userId = 1;
const query1 = `SELECT * FROM users WHERE id = ${userId}`;
const query2 = `SELECT * FROM users WHERE id = ` + userId;
const query2 = 'SELECT * FROM users WHERE id =' + userId;

const columns = 'id, name';
Users.query(`SELECT ${columns} FROM users`);
```
3 changes: 2 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module.exports = {
'tsr-detect-pseudo-random-bytes': [true],
'tsr-detect-unsafe-regexp': [true],
'tsr-disable-mustache-escape': [true],
'tsr-detect-html-injection': [true]
'tsr-detect-html-injection': [true],
'tsr-detect-sql-literal-injection': [true]
}
};
4 changes: 2 additions & 2 deletions npm-shrinkwrap.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "tslint-config-security",
"version": "1.10.0",
"version": "1.11.0",
"description": "TSLint security rules",
"main": "./index.js",
"files": [
Expand Down Expand Up @@ -42,7 +42,8 @@
"typescript": "^3.0.1"
},
"peerDependencies": {
"tslint": "^5.8.0"
"tslint": "^5.8.0",
"tslib": "^1.9.2"
},
"dependencies": {
"safe-regex": "^1.1.0"
Expand Down
58 changes: 58 additions & 0 deletions src/is-sql-query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
function createMainKeywordsPattern(keyword: string): RegExp {
return new RegExp(`(^|\\s)(${keyword})`);
}

const selectKeyword: RegExp = createMainKeywordsPattern('SELECT');
const deleteKeyword: RegExp = createMainKeywordsPattern('DELETE');
const insertKeyword: RegExp = createMainKeywordsPattern('INSERT');
const updateKeyword: RegExp = createMainKeywordsPattern('UPDATE');
const dropKeyword: RegExp = createMainKeywordsPattern('DROP');
const createKeyword: RegExp = createMainKeywordsPattern('CREATE');
const alterKeyword: RegExp = createMainKeywordsPattern('ALTER');

/**
* @description Basic SQL query detection
* @param {string} q
* @returns {boolean}
*/
export function isSqlQuery(q: string): boolean {
// detect the shortest sql query
if (!q[11]) {
return false;
}

const query: string = q.toUpperCase();

if (selectKeyword.test(query) && (query.includes(' FROM ') || query.includes('*FROM '))) {
return true;
}

if (insertKeyword.test(query) && query.includes(' INTO ')) {
return true;
}

if (updateKeyword.test(query) && query.includes(' SET ')) {
return true;
}

if (deleteKeyword.test(query) && query.includes(' FROM ')) {
return true;
}

if (dropKeyword.test(query) && (query.includes(' TABLE ') || query.includes(' DATABASE '))) {
return true;
}

if (
createKeyword.test(query) &&
(query.includes(' INDEX ') || query.includes(' TABLE ') || query.includes(' DATABASE '))
) {
return true;
}

if (alterKeyword.test(query) && query.includes(' TABLE ')) {
return true;
}

return false;
}
37 changes: 37 additions & 0 deletions src/rules/tsrDetectSqlLiteralInjectionRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import * as ts from 'typescript';
import * as Lint from 'tslint';
import {isSqlQuery} from '../is-sql-query';

export class Rule extends Lint.Rules.AbstractRule {
apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new RuleWalker(sourceFile, this.getOptions()));
}
}

const stringLiteralKinds: number[] = [ts.SyntaxKind.NoSubstitutionTemplateLiteral, ts.SyntaxKind.StringLiteral];
const generalErrorMessage: string = 'Found possible SQL injection';

class RuleWalker extends Lint.RuleWalker {
visitTemplateExpression(node: ts.TemplateExpression) {
const {parent} = node;

if (
(!parent || parent.kind !== ts.SyntaxKind.TaggedTemplateExpression) &&
isSqlQuery(node.getText().slice(1, -1))
) {
this.addFailureAtNode(node, generalErrorMessage);
}

super.visitTemplateExpression(node);
}

visitBinaryExpression(node: ts.BinaryExpression) {
const {left} = node;

if (left && stringLiteralKinds.includes(left.kind) && isSqlQuery(left.getText().slice(1, -1))) {
this.addFailureAtNode(left, generalErrorMessage);
}

super.visitBinaryExpression(node);
}
}
22 changes: 22 additions & 0 deletions test/rules/tsr-detect-sql-literal-injection/default/test.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
const userId = 1;
let query = `SELECT * FROM users WHERE id = ${userId}`;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found possible SQL injection]
query = `SELECT *FROM users WHERE id = ` + userId;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found possible SQL injection]
query = ' SELECT * FROM users WHERE id =' + userId;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found possible SQL injection]

db.query(query);

const columns = 'id, name';
Users.query( ` SELECT ${columns} FROM users` );
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Found possible SQL injection]


const query = sql`SELECT * FROM users WHERE id = ${userId}`;
db.query(query);

Users.query(`SELECT id, name FROM users`);

const punctuation = '!';
console.log(`Not SQL${punctuation}`);
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"rulesDirectory": "../../../../dist/rules",
"rules": {
"tsr-detect-sql-literal-injection": true
}
}

0 comments on commit e58c93f

Please sign in to comment.