move sanitation of urls to the serverside code to also provide security for clients, fix #151

6 files changed, 56 insertions, 10 deletions

diff --git a/db/feed.php b/db/feed.php
index 22fbc359b..2bf16fc3b 100644
--- a/db/feed.php
+++ b/db/feed.php
@@ -65,4 +65,21 @@ class Feed extends Entity implements IAPI {
 		);
 	}
 
+
+	public function setUrl($url) {
+		$url = trim($url);
+		if(strpos($url, 'http') === 0) {
+			parent::setUrl($url);
+		}
+	}
+
+
+	public function setLink($url) {
+		$url = trim($url);
+		if(strpos($url, 'http') === 0) {
+			parent::setLink($url);
+		}
+	}
+
+
 }
\ No newline at end of file
diff --git a/db/item.php b/db/item.php
index 332fd630e..1632d5438 100644
--- a/db/item.php
+++ b/db/item.php
@@ -119,5 +119,12 @@ class Item extends Entity implements IAPI {
 	}
 
 
+	public function setUrl($url) {
+		$url = trim($url);
+		if(strpos($url, 'http') === 0) {
+			parent::setUrl($url);
+		}
+	}
+
 }
 
diff --git a/templates/part.items.php b/templates/part.items.php
index f9fc3805f..9a067cf6e 100644
--- a/templates/part.items.php
+++ b/templates/part.items.php
@@ -22,7 +22,7 @@
 		<h1 class="item_heading">{{ item.title }}</h1>
 		<h1 class="item_title">
 			<a ng-click="itemBusinessLayer.setRead(item.id)"
-				target="_blank" ng-href="{{ item.url|ocSanitizeURL }}">
+				target="_blank" ng-href="{{ item.url }}">
 				{{ item.title }}
 			</a>
 		</h1>
@@ -30,7 +30,7 @@
 		<h2 class="item_author">
 			<span ng-show="itemBusinessLayer.noFeedActive() && feedBusinessLayer.getFeedLink(item.feedId)">
 				<?php p($l->t('from')) ?>
-				<a 	target="_blank" ng-href="{{ feedBusinessLayer.getFeedLink(item.feedId)|ocSanitizeURL }}"
+				<a 	target="_blank" ng-href="{{ feedBusinessLayer.getFeedLink(item.feedId) }}"
 					class="from_feed">{{ itemBusinessLayer.getFeedTitle(item.id) }}</a>
 			</span>
 			<span ui-if="item.author">
diff --git a/tests/unit/businesslayer/FeedBusinessLayerTest.php b/tests/unit/businesslayer/FeedBusinessLayerTest.php
index c16a8c4ae..8fce4ce9d 100644
--- a/tests/unit/businesslayer/FeedBusinessLayerTest.php
+++ b/tests/unit/businesslayer/FeedBusinessLayerTest.php
@@ -113,7 +113,7 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
 	}
 
 	public function testCreate(){
-		$url = 'test';
+		$url = 'http://test';
 		$folderId = 10;
 		$createdFeed = new Feed();
 		$ex = new DoesNotExistException('yo');
@@ -168,7 +168,7 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
 
 
 	public function testCreateItemGuidExistsAlready(){
-		$url = 'test';
+		$url = 'http://test';
 		$folderId = 10;
 		$createdFeed = new Feed();
 		$ex = new DoesNotExistException('yo');
diff --git a/tests/unit/db/FeedTest.php b/tests/unit/db/FeedTest.php
index 57b739bfb..10c2350f9 100644
--- a/tests/unit/db/FeedTest.php
+++ b/tests/unit/db/FeedTest.php
@@ -34,24 +34,39 @@ class FeedTest extends \PHPUnit_Framework_TestCase {
 	public function testToAPI() {
 		$feed = new Feed();
 		$feed->setId(3);
-		$feed->setUrl('url');
+		$feed->setUrl('http://google');
 		$feed->setTitle('title');
 		$feed->setFaviconLink('favicon');
 		$feed->setAdded(123);
 		$feed->setFolderId(1);
 		$feed->setUnreadCount(321);
-		$feed->setLink('link');
+		$feed->setLink('https://google');
 
 		$this->assertEquals(array(
 			'id' => 3,
-			'url' => 'url',
+			'url' => 'http://google',
 			'title' => 'title',
 			'faviconLink' => 'favicon',
 			'added' => 123,
 			'folderId' => 1,
 			'unreadCount' => 321,
-			'link' => 'link'
+			'link' => 'https://google'
 			), $feed->toAPI());
 	}
 
+
+	public function testSetXSSUrl() {
+		$feed = new Feed();
+		$feed->setUrl('javascript:alert()');
+		$this->assertEquals('', $feed->getUrl());
+	}
+
+
+	public function testSetXSSLink() {
+		$feed = new Feed();
+		$feed->setLink('javascript:alert()');
+		$this->assertEquals('', $feed->getLink());
+	}
+
+
 }
\ No newline at end of file
diff --git a/tests/unit/db/ItemTest.php b/tests/unit/db/ItemTest.php
index d48c8da12..971d808f0 100644
--- a/tests/unit/db/ItemTest.php
+++ b/tests/unit/db/ItemTest.php
@@ -71,7 +71,7 @@ class ItemTest extends \PHPUnit_Framework_TestCase {
 		$item->setId(3);
 		$item->setGuid('guid');
 		$item->setGuidHash('hash');
-		$item->setUrl('url');
+		$item->setUrl('https://google');
 		$item->setTitle('title');
 		$item->setAuthor('author');
 		$item->setPubDate(123);
@@ -88,7 +88,7 @@ class ItemTest extends \PHPUnit_Framework_TestCase {
 			'id' => 3,
 			'guid' => 'guid',
 			'guidHash' => 'hash',
-			'url' => 'url',
+			'url' => 'https://google',
 			'title' => 'title',
 			'author' => 'author',
 			'pubDate' => 123,
@@ -119,4 +119,11 @@ class ItemTest extends \PHPUnit_Framework_TestCase {
 	}
 
 
+	public function testSetXSSUrl() {
+		$item = new Item();
+		$item->setUrl('javascript:alert()');
+		$this->assertEquals('', $item->getUrl());
+	}
+
+
 }
\ No newline at end of file