Skip to content

Fix awk linter and security concerns.#1411

Merged
w0rp merged 2 commits into
dense-analysis:masterfrom
adedomin:master
Mar 14, 2018
Merged

Fix awk linter and security concerns.#1411
w0rp merged 2 commits into
dense-analysis:masterfrom
adedomin:master

Conversation

@adedomin

@adedomin adedomin commented Mar 9, 2018

Copy link
Copy Markdown
Contributor

closes #1401

  • Fixes arbitrary code execution flaw.
  • adds handler with unit test; the cpplint one was not working for me.

@w0rp w0rp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Cool, add a test for the GetCommand function too in the test/command_callback directory. Add a comment explaining that the --source argument is there to prevent executing arbitrary code.

 * Made it secure, albeit less useful.
 * Added gawk handler; the cpplint one was not working?
 * Added gawk handler test.
 * added warning to gawk handler.
 * added gawk command callback test
 * added comment about --source
@adedomin

Copy link
Copy Markdown
Contributor Author

Done, let me know if there are any other issues.

note: I made some new changes/improvements.

Comment thread ale_linters/awk/gawk.vim
\ get(g:, 'ale_awk_gawk_executable', 'gawk')

let g:ale_awk_gawk_options =
\ get(g:, 'ale_awk_gawk_options', '')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should keep these options, so people can set other options.

@@ -0,0 +1,27 @@
Before:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. Add a test here to check that you can still set other options for gawk.

@adedomin

Copy link
Copy Markdown
Contributor Author

not sure how useful the command line options are. but re-added anyhow.

@w0rp w0rp merged commit 92e6e4d into dense-analysis:master Mar 14, 2018
@w0rp

w0rp commented Mar 14, 2018

Copy link
Copy Markdown
Member

Cheers! 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Awk linter executes arbitrary code

2 participants