webschik / tslint-config-security Goto Github PK

I just read that issue Roadmap TSlint -> ESlint in which one of the maintainers of TSlint explains that TSlint will be deprecated and ESlint will scan TypeScript files in future.

As the maintainer points out this will require a lot of work but eventually will happen.

There is also a statement by the ESlint community:
https://eslint.org/blog/2019/01/future-typescript-eslint#the-future-of-typescript-on-eslint

What do you plan to do then with your project TSlint – config – security?

Hey! Thanks for making this set of rules, we're finding them really useful to have on our project.

We've run in to a tiny bug with the tsr-detect-non-literal-fs-filename rule when it runs on a toString call.

As toString exists as a function on the prototype of the fsModuleMethodsArgumentsInfo object, the rule thinks that it's found a match for fs and throws a TypeError: fsArgsInfo.filter is not a function error when it tries to lint it.

I guess more broadly this would affect any case where the name of the function being linted is a function on Object.prototype.

Code Reference

When methodName is toString, this returns the object's toString function rather than an entry from the fsModuleMethodsArgumentsInfo list:

https://github.com/webschik/tslint-config-security/blob/master/src/rules/tsrDetectNonLiteralFsFilenameRule.ts#L16

And then this tries to call filter on the object's toString function:

https://github.com/webschik/tslint-config-security/blob/master/src/rules/tsrDetectNonLiteralFsFilenameRule.ts#L20

Commit c5e22f7 moves typescript to devDependency but it's used as a dependency.

Our builds do fail because our (non-standard) build system does depend on the dependency lists of modules to be correct. This issue is related to #9, I checked that our build system will work again when typescript is set as a dependency/peerDependency again.

Commit c5e22f7 did remove dependency on tslib but it is still needed.

Our builds do fail because our (non-standard) build system does depend on the dependency lists of modules to be correct.

Instead of disabling some linter rules globally, is disabling linter rules for one file or a line of code possible? Thank you.

The title says everything. :D

const s = 'test';
const o = {
  open(a: string) {},
};
o.open(s);

will trigger on the last line

[tslint] Found fs.open with non-literal argument as index 0 (tsr-detect-non-literal-fs-filename)

I tried one, two and three arguments for method (I personally has open method with three arguments) – any of these cases triggers rule.

It will be really useful to have "more info about the issue" field in the output.

Maybe you don't want to overwhelm the default human-readable output of TSLint but maybe you can add this into the JSON output?

I see that the JSON output provides a lot of information then more info information will be really cool.

You already have good documentation about every rule so it won't be hard to do.

PS: I have seen a few linters that do that - Bandit for Python is one example and I am helping with the Gosec for golang to do just that.

tsr-detect-non-literal-fs-filename error gets triggered when nodejs __dirname is used in fs

const variableName = fs.readFileSync(__dirname + 'filename.txt`, 'utf-8')

Do you want me to make a pull request?
Thank you.

Maybe it will be a good idea to have a hardcoded credential.

There is really good documentation on projects GitHub webpage for the rules but it will be a lot nicer to have a specific webpage for every single rule.

One way to achieve this will be by using GitHub pages. GitHub pages are really easy to set up and provides you with different layout options.

tsr-detect-html-injection detects many false positives. For example my server contains this chunk of code:

import { Column, Entity, Generated, PrimaryColumn } from "typeorm";

@Entity()
export class User {
  @Column()
  @Generated("uuid")
  public uuid?: string;

  @PrimaryColumn() public sub: string;

  @Column({ nullable: true })
  public username?: string;

  constructor(sub: string) {
    this.sub = sub;
  }
}

Line 15 (this.sub = sub;) reports an error. While I see the idea of the rule, this will yield many false positives.

Our build is failing because tslib is not noted as dependency in package json.

Also TypeScript is noted as devdependency, even though it is present in a js file (src/rules/tsrDetectBufferNoassertRule.js). It should be set as dependency or it should not be present in the js file. It is probably in the js file because of line 64 of src/rules/tsrDetectBufferNoassertRule.ts.

There are different issues - some are more concrete others are problems only in specific situations and easily can produce many false-positives.

Like tsr-detect-non-literal-fs-filename rule is a problem when there is a user input which is not checked. This is a narrow case from all possible uses of fs.open/fs.read/fs.write and can easily produce many false-positives.

Having a confidence level will give an understanding of how possible is a false-positive from a concrete rule.

Many static code analysis tools for security vulnerabilities like Bandit and Gosec are using "confidence" level in their output.

Hi,

first: thanks for this great TSLint plugin.

My question is if this plugin is (fully) compatible with Typescript 3.x? The package.json specifies ^2.9.1 as requested version.

Hey there,

i saw you accepted my pull request - thank you! :)
Could you publish the new version 2.0 to npm so I can use it in the libraries I'm currently updating?

Hi,

I noticed #4, but I am having issues getting the plugin to work with Typescript 3. The rules seem to be loaded just fine, but they never actually execute.

Steps to reproduce:

• Generate new project with typescript 3.0.3 and tslint 5.11.0. Write a file that blatantly goes against one of the rules in tslint-config-security. Make sure tslint.json extends tslint-config-security.
• Clone tslint-config-security in an adjacent directory
• npm install in tslint-config-security
• npm link in tslint-config-security
• npm link tslint-config-security in your project
• Run tslint. Notice no errors are present (or at least, none from tslint-config-security)
• Go into the local clone of tslint-config-security, change the typescript version to 3.1.2
• npm install in tslint-config-security
• Run tslint. Notice it now shows errors from tslint-config-security

This might also be an issue with tslint or with typescript. I haven't had an opportunity to dig much further into it.

I am working on a repo where I just introduced the tslint-config-security TSLint rules. Immediately afterwards, all my external Git tooling -- specifically, Tower -- started failing with this error message:

Husky requires Node 6, can't run Git hook.

Upon further investigation, this is what I believe happened:

• My system Node version is 4.x. My NVM Node version is 10.x.
• Upon installing tslint-config-security, the Git hooks were installed. I admit, I'm still unsure the exact mechanism by which this happened. I believe it was through Husky's post-install script, but I cannot confirm.
• Tower -- per this docs page -- by default runs Git hooks with the user's default environment, as if they had started a new shell.
• This means Tower runs this project's Git hooks under the system Node version of 4.x.
• Husky breaks under Node 4.x.

This was difficult and confusing to deal with for a number of reasons:

• I was not expecting any additional changes or side effects from installing the package.
• The failure manifests far away from where it originated.
• The error message refers to something different from and seemingly unrelated to the real root cause.

I would recommend instead, the Git hooks be included with the project, but opt-in rather than opt-out.

Please consider this change, as it could save a users quite a lot of confusion and frustration. If you agree that this is the correct course of action, I would be happy to contribute the change as a pull request. Thank you in advance.

EDIT:

I should clarify, I strongly suspect that there are many different ways to encounter analogous failures from these Git hooks, and they will be equally confusing. While an outdated system Node version, specifically, is probably not too common, I strongly suspect that the sum total of the ways in which this could break add up to reasonably common on the whole.