Recommend setting base_url

CommentFileSizeAuthor
#10 2001092-security_review-base_url-9.patch4.05 KBcoltrane
#9 2001092-security_review-base_url-9.patch0 bytescoltrane
#4 2001092.patch896 bytescoltrane
#1 2001092-security_review-base_url.patch2.87 KBcoltrane

Comments

coltrane’s picture

coltrane
he/him
commented
Status: Active » Needs review
StatusFileSize
new 2001092-security_review-base_url.patch2.87 KB
coltrane’s picture

coltrane
he/him
commented
Status: Needs review » Fixed

Committed http://drupalcode.org/project/security_review.git/commit/2244ea3

coltrane’s picture

coltrane
he/him
commented
Status: Fixed » Needs work

Re-opening to tokenize settings.php instead of including.

coltrane’s picture

coltrane
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new 2001092.patch896 bytes
coltrane’s picture

coltrane
he/him
commented
Status: Needs review » Fixed

committed http://drupalcode.org/project/security_review.git/commit/1a7b0ace6c870b4...

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

FWIW, I'd personally prefer the include style check. There's a chance that the tokenize test will succeed in a scenario like:

if (FALSE) {
  $base_url = 'foo';
}

And so it would be a false positive.

coltrane’s picture

coltrane
he/him
commented
Status: Fixed » Active

Re-opening based on #6 for further thought.

coltrane’s picture

coltrane
he/him
commented
Title: Check for base_url in settings.php » Include settings.php as additional way to check for base_url being set
coltrane’s picture

coltrane
he/him
commented
Status: Active » Needs review
StatusFileSize
new 2001092-security_review-base_url-9.patch0 bytes

This adds per-check settings on the settings page. You can flag this check to use 'include' method.

coltrane’s picture

coltrane
he/him
commented
StatusFileSize
new 2001092-security_review-base_url-9.patch4.05 KB

last patch had errors on upload, trying again

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Looks great to me on visual review.

coltrane’s picture

coltrane
he/him
commented
Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Needs review » Patch (to be ported)

Committed. http://drupalcode.org/project/security_review.git/commit/f577b38

  • coltrane committed 2244ea3 on 8.x-1.x 
    Issue #2001092 by coltrane: Added Check for base_url() in settings.php.
  • coltrane committed 1a7b0ac on 8.x-1.x
    Followup to Issue #2001092 by coltrane: Tokenize settings.php for...
  • coltrane committed f577b38 on 8.x-1.x
    Followup #2001092 by coltrane: Add include settings.php as additional...
smustgrave’s picture

smustgrave commented
Issue summary: View changes
Status: Patch (to be ported) » Closed (outdated)

As Drupal6 has been EOL https://www.drupal.org/about/drupal6-eol closing as outdated