Результаты секьюрити сканера

А нафига вообще все это писать???
[code]
String IdDon = ApexPages.currentPage().getParameters().get('id');
IdDon = String.escapeSingleQuotes(IdDon);
donId = Id.valueOf(IdDon);
[/code]

Тот же самый эффект будет от одной строчки

[code]
    Id donId = ApexPages.currentPage().getParameters().get('id');
[/code]

Возможно и сканер перестанет ругаться

Это тоже нафиг не нужно

[code]
if(donId != null && donId != '')
[/code]

Переменная с типом Id не может содержать '' (пустую строку) 

Все это можно сократить до

[code]
if(Schema.sObjectType.Donation__c.isAccessible() && Schema.sObjectType.Donation__c.isDeletable()) {
    delete [SELECT Id FROM Donation__c WHERE Id = :ApexPages.currentPage().getParameters().get('id')];
}     
[/code]
Кстати не знаю как где, но вроде в java {} ставятся на той же строке где и if/for ...
Знаю что в php принято с новой строки - глаз мозолит пипец.

И совет еще по оформлению - обрати внимание как SELECT/FROM/WHERE написаны - так все пишут на Apex. Тоже сразу видно кто пишет все в нижнем регистре.

вот это несколько параноидальный подход:

[quote="DevNull"]

[code]id donId;
            try
            {
                String IdDon = ApexPages.currentPage().getParameters().get('id');
                IdDon = String.escapeSingleQuotes(IdDon);
                donId = Id.valueOf(IdDon);
            }
            catch(Exception e)
            {
                donId = null;
            }
            if(donId != null && donId != '')
            {[/code]
[/quote]

но у меня тоже раньше бывали приступы паранои и я делал что-то вроде того.

поэтому вопрос к зрителям:
[i]как кратко и безопасно принять ID из УРЛ параметра, так чтобы обработать все "засады", как нулевой параметр и злонамеренный пользовательский ввод?[/i]

[quote="Dmitry Shnyrev"]А нафига вообще все это писать???

String IdDon = ApexPages.currentPage().getParameters().get('id');
IdDon = String.escapeSingleQuotes(IdDon);
donId = Id.valueOf(IdDon);

Тот же самый эффект будет от одной строчки

Id donId = ApexPages.currentPage().getParameters().get('id');[/quote]
Так а если там чушь будет? Надо ж проверить.

[quote="Dmitry Shnyrev"]Кстати не знаю как где, но вроде в java {} ставятся на той же строке где и if/for ... [/quote]
+

[quote="Dmitry Shnyrev"]И совет еще по оформлению - обрати внимание как SELECT/FROM/WHERE написаны - так все пишут на Apex.[/quote]
+

Давайте не будем придираться к самому написанию, тут криво не спорю(и еще как криво).


[quote="Dmitry Shnyrev"]Тот же самый эффект будет от одной строчки

Id donId = ApexPages.currentPage().getParameters().get('id');[/quote]
Все верно! Так оно и было. И ругается именно на получение данного параметра. 
Вопрос как решить то проблему. 
[quote="Den Brown"]как кратко и безопасно принять ID из УРЛ параметра, так чтобы обработать все "засады", как нулевой параметр и злонамеренный пользовательский ввод?[/quote]
Вот в этом и есть проблема мне кажется. От он и ругается. 

Я вот порыскал немного и 
[code]Id donId = ApexPages.currentPage().getParameters().get('id');[/code]
походу вполне достаточно.

[quote="Andrew Muzychuk"]походу вполне достаточно.[/quote]
Достаточно для чего?
Этого достаточно чтобы вызвать ошибку при секьюрити сканнинге. 
Так как мне этого избежать? Гугль не помогает. 

Я так понял, что ругается на [code]String IdDon = ApexPages.currentPage().getParameters().get('id');
                IdDon = String.escapeSingleQuotes(IdDon);
                donId = Id.valueOf(IdDon);[/code], а конкретно на [code]
                IdDon = String.escapeSingleQuotes(IdDon);
                donId = Id.valueOf(IdDon);[/code] ибо это через чур. У меня подозрение, что надо [code]
                IdDon = String.escapeSingleQuotes(IdDon);
                donId = Id.valueOf(IdDon);[/code] убрать, чтоб никто не ругался.

А есть где-то инструкция/документация этого сканера? Описание ошибок?

[quote="Den Brown"]как кратко и безопасно принять ID из УРЛ параметра, так чтобы обработать все "засады", как нулевой параметр и злонамеренный пользовательский ввод?[/quote]
Я же уже ответил - ответ в типе присваиваемое переменой. 
Попробуй сам что-нибудь левое запихнуть в переменную с типом Id
Это уже первый шаг которые решает 99% проблем с фильтрацией пользовательских данных. 
Второй это сам запрос. Если даже тебе пришла валидная Id в запросе и код не свалился в exception при присвоении переменной, то следующий SOQL вернет тебе пустой List если ты передал не тот Id.
А дальше я написал еще короче в одну строку - 
Id = :some_var
Это, дай бох мне памяти, в нормальных языках программирования называется подготовленный запрос с биндингом. Он уже экранирует все что попадает в переменную. И не надо бояться что кто-то туда засунет зловредный код потому что он не выполнится (не станет частью запроса). 
А вот уже если вы пытаетесь сделать так:
[code]
String q = 'SELECT Id FROM SomeObject WHERE Id = \''+ApexPages.currentPage().getParameters().get('id')+'\'';
Database.query(q);
[/code]
Вот тогда молитесь!

Это касательно матчасти.

DevNull, на счет сканера. Сложно ответить почему он у тебя ругается. Надо видеть какой из вариантов ты ему скармливаешь. 
Я уже сто лет делаю как описал выше и не помню чтобы испытывал какие-то проблемы.

Как вариант - возьми мое решение с инициализацией Id переменной и без нее, скорми сканеру и выложи результат. 

Если не прокатит - то будем копать глубже. 

Да, я дал не совсем правильный пример, он вызывает много вопросов. Чуть позже вышлю первоначальный вариант. 

[code]
String donId = ApexPages.currentPage().getParameters().get('id');
if(donId != null && donId != '')
{
	if(Schema.sObjectType.Donation__c.isAccessible())
	{
		list<Donation__c> donList = [select id from Donation__c where id=: donId];
		if(Schema.sObjectType.Donation__c.isDeletable())
		{                    
			delete donList;
		}      
	}      
}   
[/code]
Ошибка сканера:
Query: XSRF
Object: get in file: /classes/Donation_registrCtrl.cls
L 51: [code]String donId = ApexPages.currentPage().getParameters().get('id');[/code]
Object: donid in file: /classes/Donation_registrCtrl.cls
L 56: [code]list<Donation__c> donList = [select id from Donation__c where id=: donId];[/code]
Object: delete in file: /classes/Donation_registrCtrl.cls
L 59: [code]delete donList;[/code]

Автор, извини, что ушли в офф-топ, но все же интересная тема

[quote="Dmitry Shnyrev"]А вот уже если вы пытаетесь сделать так: 
String q = 'SELECT Id FROM SomeObject WHERE Id = \''+ApexPages.currentPage().getParameters().get('id')+'\'';
Database.query(q);

Вот тогда молитесь![/quote]

это понятно,

скажи, а что если вот так:

[code]String donId = ApexPages.currentPage().getParameters().get('id');[/code]

и потом сразу ее подаю в квери вот так:

[code][SELECT Id FROM Donation__c WHERE Id = :donId];[/code]
есть риск инъекции?

а что если стринговая :donId пришла как ПОСТ параметр, автоматом присваемый к какой то переменной контроллера? есть риск инъекции?

кстати, в моем понимании вот это:

[code][SELECT Id FROM Donation__c WHERE Id = :ApexPages.currentPage().getParameters().get('id')];[/code]

не сильно то отличается от вот этого:

[code]String q = 'SELECT Id FROM SomeObject WHERE Id = \''+ApexPages.currentPage().getParameters().get('id')+'\'';[/code]

[quote="Den Brown"]есть риск инъекции?[/quote]
Нет

[quote="Den Brown"]не сильно то отличается от вот этого:[/quote]
Отличается сильно.

Попробуй для саморазвития погуглить тему SQL injection НЕ в разрезе Apex - ее основы сразу позволят понять в чем отличие этих двух строк кода.

Все, я понял блин в чем проблема. 
Не вчитался в результаты сканера. Там же все написано (надо смотреть сочетание всех 3-х строчек, а не только первой)

[code]
Ошибка сканера: 
Query: XSRF 
Object: get in file: /classes/Donation_registrCtrl.cls 
L 51:
[b]String donId = ApexPages.currentPage().getParameters().get('id');[/b]
Object: donid in file: /classes/Donation_registrCtrl.cls 
L 56:
[b]list<Donation__c> donList = [select id from Donation__c where id=: donId];[/b]
Object: delete in file: /classes/Donation_registrCtrl.cls 
L 59:
[b]delete donList;[/b]
[/code]

Вот тут чувак все правильно написал - 
https://developer.salesforce.com/forums?id=906F000000097qiIAA
Вчитайся в то что написано в подсвеченной зеленым.

Это более глобальная проблема. Апдейт данных в базе на основе пaрaметров из URL.

Так как бы делать нельзя с точки зрения безопасности. 

К примеру я смогу сделать спец ссылку для удаление и послать ее пользователю. Тот просто кликнув по ней удалит то что мне надо от своего имени и под своими привилегиями.

Тут сказано что не надо искать обход проблемы - надо менять подход.

Надо делать чтобы метод вызывался POST запросом. Т.е. в случае с VF повесить вызов на кнопку или JS 

Странно что светилы нашего форума не поправили нас сразу :D
Но с другой стороны может и к лучшему - дали возможность разобраться самим :D :D :D 

[quote="Dmitry Shnyrev"]Надо делать чтобы метод вызывался POST запросом. Т.е. в случае с VF повесить вызов на кнопку или JS[/quote]

вот это тоже интересная тема. 

т.е. с ГЕТ запроса не позволяется выполнять что то серьезное, во избежании описанной ситуации. 

что то серьезное нужно явно вызывать нажатием кнопки, у которой в свою очередь могут быть самые разные механизмы работы

Это плохо что никак не обойти. Это откатит проект на много назад. 

[quote="DevNull"]Это откатит проект на много назад.[/quote]
Как то сильно сказано :D Мне кажется тут намного все проще.

А можно, может глупый или странный, вопрос - а что за сканер такой и зачем им так заморачиваться?

[quote="Andrew Muzychuk"]а что за сканер такой[/quote]
Ну это вопрос действительно крайне странный :D
Если не сталкиваться, то хотя бы слышать про него должен был по любому.
https://security.secure.force.com/security/tools/forcecom/scanner

Та я все периодически тут встречаю упоминание этого (я так подозреваю, он один) сканера, но самому никогда не надо было им пользоваться. Ну, точнее, меня никто не просил, я и не смотрел.

Да собственно мало кто им пользуется.
Обязательно с ним столкнешься если будешь проходить security review для AppExchange.
А в остальном чисто поиграться. Клиенты тоже не особо про него слышали и тем более не в курсе что с ним делать.
Если есть время можно воспользоваться для самоконтроля. Вроде там есть бесплатная квота.

Да, есть. На количество строк. А я его как-то запускал. Пришел большой архив :-) Я его забросил, ибо нет времени :-)

[quote="Dmitry Shnyrev"]Клиенты тоже не особо про него слышали и тем более не в курсе что с ним делать. [/quote]
А моей ситуации как раз где то услышали( И понеслась... 

[quote="DevNull"]Как победить? [/quote]

C такой обработкой как у вас
1 - эскейп скобок
2 - приведение к типу ID
(хотя просто приведения к Id было бы достаточно)
все должно быть [b]хорошо[/b] - это [i]ложное срабатывание[/i] сканера (т.н. False Positive)

При прохождении секурити ревью надо к репорту сканера добавить еще документ с перечислением ложных срабатываний и кратким их пояснением - чтобы human потом посмотрел и принял.

Мне лень писать такого рода документы - поэтому я айдишки из урла беру стандартным контроллером  - который не вызывает срабатывания сканера.

[quote="yurybond"]2 - приведение к типу ID [/quote]
пробывал не прокатило
[quote="yurybond"]все должно быть хорошо - это ложное срабатывание сканера (т.н. False Positive)
При прохождении секурити ревью надо к репорту сканера добавить еще документ с перечислением ложных срабатываний и кратким их пояснением - чтобы human потом посмотрел и принял.[/quote]

Секьрити ревью не будет( Заказчик услышал про сканер и все, понеслось - должно пройти! Должно без ошибок!!