The purpose of this commit is that without quote, if variable contains
characters meaningful for shell, such as space, '$', ';' they are
interpreted. The quoting avoid this interpretation.
Details
Details
- Reviewers
dereckson Sandlayth - Commits
- rDPHABc0dec7d81d77: Add quotes to command arguments
Diff Detail
Diff Detail
- Repository
- rDPHAB Docker image for Phabricator
- Lint
No Lint Coverage - Unit
No Test Coverage - Branch
- arcpatch-D603
- Build Status
Buildable 891 Build 1032: arc lint + arc unit
Event Timeline
| files/usr/local/bin/setup-phabricator | ||
|---|---|---|
| 13 | at least semicolon (;) are valid on url and a lot of meta shell chars (I don't have the list off my head.) see comment line 22 for rest of answer | |
| 22 | Additional check to proper format would be welcome. Additional commit can be added However there is nothing to lose to proper escaping arguments in using quote even if we are 100% sure it will not happen. It is even very good programming practice. One particular reason is that now we might know that there can't be shell injection, but whoever know in the future. | |
Comment Actions
A way to conduct testing
set $MYSQL_ENV_MYSQL_ROOT_PASSWORD and others variables to nothing; echo dangerous command
in previous version the line should be echoed.
in new version nothing should appear.
It could be automatized for non regression
This comment was removed by xcombelle.
This comment was removed by xcombelle.
| files/usr/local/bin/setup-phabricator | ||
|---|---|---|
| 6 | Maybe add a commentary: # mysql.host variable is fixed and should always be set as 'mysql', according to https://devcentral.nasqueron.org/diffusion/DPHAB/ : # > a MySQL container linked as 'mysql' (--link <your MySQL container>:mysql), which could be the official MySQL image or our nasqueron/mysql image, optimized for Phabricator. | |