Merge pull request #81 from jessedobbelaere/fix-double-encoding

Fix double encoding issue on text fields
forkcms · May 15, 2021 · 8b4429f · 8b4429f
2 parents 8a81581 + 2ed6933
commit 8b4429f
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 1 deletion.
diff --git a/spoon/form/text.php b/spoon/form/text.php
@@ -788,7 +788,8 @@ public function parse($template = null)
 		if($this->attributes['name'] == '') throw new SpoonFormException('A name is required for a textfield. Please provide a name.');
 
 		// start html generation
-		$output = '<input value="' . SpoonFilter::htmlspecialchars($this->getValue()) . '"';
+		// note: no need to encode the value here, it gets encoding in the getter as long as $allowHTML=true
+		$output = '<input value="' . $this->getValue() . '"';
 
 		// add attributes
 		$output .= $this->getAttributesHTML(array('[id]' => $this->attributes['id'], '[name]' => $this->attributes['name'], '[value]' => $this->getValue())) . ' />';

diff --git a/spoon/tests/form/SpoonFormDateTest.php b/spoon/tests/form/SpoonFormDateTest.php
@@ -168,4 +168,20 @@ public function testDateFormatsShort()
 		);
 		$this->loopOverFormats($formats);
 	}
+
+	public function testParse()
+	{
+		$_POST['date'] = '12/10/2026';
+		$this->assertEquals(
+			'<input type="text" value="12/10/2026" id="date" name="date" maxlength="10" data-mask="dd/mm/yy" class="inputDatefield" />',
+			$this->txtDate->parse()
+		);
+
+		// Make sure we encode XSS payloads
+		$_POST['date'] = '12/10/2026\'"()%26%25<yes><ScRiPt%20>alert(1)</ScRiPt>';
+		$this->assertEquals(
+			'<input type="text" value="12/10/2026&#039;&quot;()%26%25&lt;yes&gt;&lt;ScRiPt%20&gt;alert(1)&lt;/ScRiPt&gt;" id="date" name="date" maxlength="10" data-mask="dd/mm/yy" class="inputDatefield" />',
+			$this->txtDate->parse()
+		);
+	}
 }
diff --git a/spoon/tests/form/SpoonFormHiddenTest.php b/spoon/tests/form/SpoonFormHiddenTest.php
@@ -51,4 +51,21 @@ public function testGetValue()
 		$_POST['hidden'] = array('foo', 'bar');
 		$this->assertEquals('Array', $this->hidHidden->getValue());
 	}
+
+	public function testParse()
+	{
+		$_POST['form'] = 'hiddenfield';
+		$_POST['hidden'] = 'But I am le tired';
+		$this->assertEquals(
+			'<input type="hidden" value="But I am le tired" id="hidden" name="hidden" />',
+			$this->hidHidden->parse()
+		);
+
+		// Make sure we encode XSS payloads
+		$_POST['hidden'] = 'But I am le tired\'"()%26%25<yes><ScRiPt%20>alert(1)</ScRiPt>';
+		$this->assertEquals(
+			'<input type="hidden" value="But I am le tired&amp;#039;&amp;quot;()%26%25&amp;lt;yes&amp;gt;&amp;lt;ScRiPt%20&amp;gt;alert(1)&amp;lt;/ScRiPt&amp;gt;" id="hidden" name="hidden" />',
+			$this->hidHidden->parse()
+		);
+	}
 }
diff --git a/spoon/tests/form/SpoonFormPasswordTest.php b/spoon/tests/form/SpoonFormPasswordTest.php
@@ -114,4 +114,21 @@ public function testGetValue()
 		$_POST['name'] = array('foo', 'bar');
 		$this->assertEquals('Array', $this->txtPassword->getValue());
 	}
+
+	public function testParse()
+	{
+		$_POST['form'] = 'passwordfield';
+		$_POST['name'] = 'But I am le tired';
+		$this->assertEquals(
+			'<input type="password" value="But I am le tired" id="name" name="name" class="inputPassword" />',
+			$this->txtPassword->parse()
+		);
+
+		// Make sure we encode XSS payloads
+		$_POST['name'] = 'But I am le tired\'"()%26%25<yes><ScRiPt%20>alert(1)</ScRiPt>';
+		$this->assertEquals(
+			'<input type="password" value="But I am le tired&#039;&quot;()%26%25&lt;yes&gt;&lt;ScRiPt%20&gt;alert(1)&lt;/ScRiPt&gt;" id="name" name="name" class="inputPassword" />',
+			$this->txtPassword->parse()
+		);
+	}
 }
diff --git a/spoon/tests/form/SpoonFormTextTest.php b/spoon/tests/form/SpoonFormTextTest.php
@@ -336,4 +336,28 @@ public function testChainingMethods()
 			$this->txtName->getErrors()
 		);
 	}
+
+	public function testParse()
+	{
+		$_POST['form'] = 'textfield';
+		$_POST['name'] = 'But I am le tired';
+		$this->assertEquals(
+			'<input value="But I am le tired" id="name" name="name" type="text" class="inputText" />',
+			$this->txtName->parse()
+		);
+
+		// Make sure we encode XSS payloads
+		$_POST['name'] = 'But I am le tired\'"()%26%25<yes><ScRiPt%20>alert(1)</ScRiPt>';
+		$this->assertEquals(
+			'<input value="But I am le tired&#039;&quot;()%26%25&lt;yes&gt;&lt;ScRiPt%20&gt;alert(1)&lt;/ScRiPt&gt;" id="name" name="name" type="text" class="inputText" />',
+			$this->txtName->parse()
+		);
+
+		// Make sure we do not do double encoding on the ampersand
+		$_POST['name'] = 'Something & something else';
+		$this->assertEquals(
+			'<input value="Something &amp; something else" id="name" name="name" type="text" class="inputText" />',
+			$this->txtName->parse()
+		);
+	}
 }
diff --git a/spoon/tests/form/SpoonFormTimeTest.php b/spoon/tests/form/SpoonFormTimeTest.php
@@ -92,4 +92,21 @@ public function testGetValue()
 		$_POST['time'] = array('foo', 'bar');
 		$this->assertEquals('Array', $this->txtTime->getValue());
 	}
+
+	public function testParse()
+	{
+		$_POST['form'] = 'timefield';
+		$_POST['time'] = '15:30';
+		$this->assertEquals(
+			'<input type="text" value="15:30" id="time" name="time" maxlength="5" class="inputTimefield" />',
+			$this->txtTime->parse()
+		);
+
+		// Make sure we encode XSS payloads
+		$_POST['time'] = '15:30\'"()%26%25<yes><ScRiPt%20>alert(1)</ScRiPt>';
+		$this->assertEquals(
+			'<input type="text" value="15:30&#039;&quot;()%26%25&lt;yes&gt;&lt;ScRiPt%20&gt;alert(1)&lt;/ScRiPt&gt;" id="time" name="time" maxlength="5" class="inputTimefield" />',
+			$this->txtTime->parse()
+		);
+	}
 }