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

Fix #329, resolve osfileapi.c lgtm issue #332

Closed
wants to merge 1 commit into from
Closed

Fix #329, resolve osfileapi.c lgtm issue #332

wants to merge 1 commit into from

Conversation

avan989
Copy link
Contributor

@avan989 avan989 commented Dec 26, 2019

Describe the contribution
Fixes #329
Resolve osfileapi.c lgtm issue.

Testing performed
Steps taken to test the contribution:

  1. Build dummy repository against lgtm
  2. Verify it fixes.

Expected behavior changes
A clear and concise description of how this contribution will change behavior and level of impact.

  • execl() will only execute for /bin/sh.

System(s) tested on:

  • Hardware
  • Ubuntu 18.04
  • CFE 6.6

Contributor Info
Anh Van, NASA Goddard

@skliper
Copy link
Contributor

skliper commented Dec 30, 2019

Considering removing the shell code completely (major security issue). Will discuss in CCB.

@skliper skliper added this to the 5.1.0 milestone Dec 30, 2019
@skliper
Copy link
Contributor

skliper commented Jan 8, 2020

CCB 20190108 - Decision to retain functionality for now, discussed potential optional compile (like network code). The change in this pull request should be reviewed by CCB to improve security.

@skliper skliper mentioned this pull request Jan 8, 2020
Copy link
Contributor

@skliper skliper left a comment

Choose a reason for hiding this comment

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

Likely can remove the shell variable, if no longer applies now that it's hard coded.

Suggest wait for CCB review (may have other changes)

@skliper
Copy link
Contributor

skliper commented Jan 15, 2020

CCB 20200115 - Discussion is to split this shell code out, and optionally included

@skliper
Copy link
Contributor

skliper commented Jan 21, 2020

See #354, which will leave as is but make optional.

@skliper skliper removed this from the 5.1.0 milestone Jan 21, 2020
@skliper skliper closed this Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lgtm warning - misc.
2 participants