Skip to content

Commit

Permalink
csrf: do not embed id twice
Browse files Browse the repository at this point in the history 
In fact, it is unused so this should work.  Needs peer review.

PR: https://forum.opnsense.org/index.php?topic=6631.0
  • Loading branch information
@fichtner
fichtner committed Dec 14, 2017
1 parent ac6a1ef commit efb7013
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/www/csrf.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class LegacyCSRF
}
if ($this->is_html_output) {
$csrf = $this->newToken();
$inputtag = "<input type=\"hidden\" id=\"__opnsense_csrf\" name=\"{$csrf['key']}\" value=\"{$csrf['token']}\" />";
$inputtag = "<input type=\"hidden\" name=\"{$csrf['key']}\" value=\"{$csrf['token']}\" />";
$buffer = preg_replace('#(<form[^>]*method\s*=\s*["\']post["\'][^>]*>)#i', '$1' . $inputtag, $buffer);
// csrf token for Ajax type requests
$script = "
Expand Down

2 comments on commit efb7013

@fichtner
Copy link
Member Author

@fichtner fichtner commented on efb7013 Dec 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@AdSchellevis @fabianfrz please review: multiple forms can cause multiple injections, leading to multiple definitions of the same id. as far as I could see the id is unused.

@AdSchellevis
Copy link
Member

@AdSchellevis AdSchellevis commented on efb7013 Dec 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@fichtner I think it's safe to delete, this seems to happen with multiple <form> entries in the document, if we want a way to reference the element, we could always add a data element later on (not needed now, if issues appear, which I don't expect, we should match the element differently).

Please sign in to comment.