Skip to content

Conversation

@colinmollenhour
Copy link
Member

@colinmollenhour colinmollenhour commented Jul 31, 2022

This isn't meant to be a full audit of the .htaccess file or security of access to non-public files but currently a vanilla installation exposes the .git directory which if nothing else throws up a red flag for common scanners.

Rather than blocking specific files and directories like .git, .github, etc this will block all dot files (edit: only in the base path) which I think is a very sane out of the box configuration.

This isn't meant to be a full audit of the .htaccess file or security of access to non-public files but currently a vanilla installation exposes the .git directory which if nothing else throws up a red flag for common scanners.

Rather than blocking specific files and directories like .git, .github, etc this will block all dot files which I think is a very sane out of the box configuration.
@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

Flyingmana
Flyingmana previously approved these changes Jul 31, 2022
tmewes
tmewes previously approved these changes Jul 31, 2022
@addison74
Copy link
Contributor

addison74 commented Jul 31, 2022

The RedirectMatch directive belongs to module Alias. If it is not enabled it will be a problem because it will not work and not all users have root access. If you keep it please surround the directive with <IfModule mod_alias.c>...</IfModule mod_alias.c>.

I would choose to use the RewriteCond/RewriteRule combination because the Rewrite module is obviously enabled, being mandatory for OpenMage to function properly.

RewriteEngine on
RewriteCond %{REQUEST_URI} /\.
RewriteRule ^(.*)$ / [R=404,L]

@colinmollenhour colinmollenhour dismissed stale reviews from tmewes and Flyingmana via d32ff02 August 1, 2022 21:16
@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@colinmollenhour
Copy link
Member Author

@addison74 I changed it as you suggested so that mod_alias is not a dependency - the dot files will not be protected if someone is not using mod_rewrite which is a bit hazardous but I would venture to guess that almost nobody runs Magento without mod_rewrite so this is the best way to add protection without risking a BC break.

@addison74
Copy link
Contributor

addison74 commented Aug 1, 2022

Which variant would be better?

^(.*)$ / [R=404,L] or (.*)$ - [R=404,L]

"- (dash). A dash indicates that no substitution should be performed (the existing path is passed through untouched). This is used when a flag (see below) needs to be applied without changing the path."

I tested both variants and the result is the same. URL does not change what I wanted to happen, but maybe it would be better to use dash instead of /.

@colinmollenhour
Copy link
Member Author

colinmollenhour commented Aug 3, 2022

Which variant would be better?

I think it doesn't matter because both will match any string and the goal is simply to short-circuit the request and cause a 404 response.

@fballiano fballiano merged commit 18e68a2 into 1.9.4.x Aug 3, 2022
@fballiano fballiano deleted the block-dot-files branch August 3, 2022 16:41
@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2022

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
7 runs  ±0  5 ✔️ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit 18e68a2. ± Comparison against base commit 1699468.

sreichel added a commit that referenced this pull request Aug 12, 2022
* Merge PR #2342

* Revert "Add basic text for Ukraine (#2074)" (#2325)

This reverts commit 33dfa26.

* Mage_Catalog_Model_Product_Attribute_Backend_Groupprice_Abstract: avoid loading all websites when using only the current one (#2351)

* Added support for HTTP2 to Mage_HTTP_Client_Curl (#1137)

* Blocked access to all dot files (#2349)

* Capitalization Adjustment Regarding CamelCase in Method Names (#2365)

* refactor: Adjusted capitalization of two public methods.

* refactor: Adjusted capitalization of one protected method.

* refactor: Adjusted capitalization of where call.

* chore: Removed fixed error from phpstan baseline.

* Some microoptimization (#2335)

* Avoid duplicate method calls

* Replaced array_push()

* Changed substr() third parameter

* Use array_key_exists()

* php7 opcode - internal functions

* Enclosed error with <pre> tag for prettier error print (if developer mode is enabled). (#2368)

* Updated phpstan to 1.8.2 (#2367)

* Escape product titles in MSRP JavaScript (#2366)

* Product names were not escaped. If contained a double quote, would break the JavaScript for MSRP/MAP

* update contribution list

* Update boxes.css (#2330)

* Force describeTable() to use read DB adapter (#2371)

* Do not install n98/n98_layouthelper (#2373)

* Add apt update to XML validation workflow (#2376)

* Merged PR #2375

* Replace remaining "sizeof" calls with "count" (#2369)

* Remove DISCLAIMER and change Magento -> OpenMage in header (#2297)

* Added label for phpstan cosmetic changes (#2384)

* Added weight to salesOrderShipmentAddTrack API (#1377)

* PHPStan/DOCBlock fixes (#2336)

* Updated docs for email addTo() (#2382)

* Updated phpstan experimental (#2386)

* Cosmetic changes to Mage_Payment_Model_Method_Abstract::validate() (#2388)

* Replaced join() calls with implode() (#2389)

* Hidden empty sub menu from backend (#2391)

* Remove Thumbs.db file (#2394)

* Support PHP 8.1 in composer.json (#2378)

* php condition in composer.json

This solves the issue related to php versions > 8.1

* Reduced condition for PHP requirement

* Changed PHP requirement

* Updated version in Ubuntu 22.04 based on PHP 8.1.2

* Composer.lock updated in Ubuntu 20.04 (PHP 8.1.2)

* Update composer.lock

* Update composer.lock

* Blocked various file types in .htaccess (#2359)

* Color swatches work with disparate product IDs (#2390)

* Move Credit Memo at the end of the buttons list (#2392)

* Version bump (#2387)

* Minor fixes on 'filter_condition_callback' method _filterStoreCondition() (#2362)

* add ReturnTypeWillChange to various Files catched by code style checker #2302

* Phpstan fixes (#2396)

* Fixed addCrumb()

* Fixed initForm() and _needToAddDummy()

* Fixed addLink()

* Fixed addLinkRel()

* Fixed canUseCanonicalTag()

* Fixed getAddUrl...()

* Fixed rollBack() camelCase error reported by phpstan (#2403)

* Changes default root dir in composer.json (#2401)

* Fixed targetNamespace for WS-I Compliant SOAP APIs (#2405)

* Updated phpstan baseline

Co-authored-by: sv3n <[email protected]>
Co-authored-by: Fabian Blechschmidt <[email protected]>
Co-authored-by: Colin Mollenhour <[email protected]>
Co-authored-by: Kevin Jakob <[email protected]>
Co-authored-by: Ng Kiat Siong <[email protected]>
Co-authored-by: Scott Moore <[email protected]>
Co-authored-by: ADDISON <[email protected]>
Co-authored-by: Justin Beaty <[email protected]>
Co-authored-by: luigifab <[email protected]>
Co-authored-by: Daniel Fahlke <[email protected]>
Co-authored-by: leissbua <[email protected]>
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.

5 participants