2014-12-02 2 views
1

Я читаю файл, используя что-то вроде:вход проверки с стандом :: ifstream

std::ifstream is(file_name); 
    std::string s; 

    if(!is.good()) { 
     std::cout << "error opening file: " << file_name << std::endl; 
    } else { 
     while(!is.eof()) { 
      s.clear(); 
      is >> s; 
      if (s.empty()) continue; 
      if (s.size() < 1 || s.size()>0x7FFFFFFF) { 
       std::cout << "implausible data" << std::endl; 
       continue; 
      } 
      char *ss = new char[ s.size() + 1 ]; // COVERITY bails out 
      // do something with the data 
      delete[]ss; 
     } 
    } 

Когда я анализирую выше код с статическим анализом кода инструментом coverity (бесплатная версия), линия отмечена Coverity вываливается выдает ошибку:

Untrusted value as argument (TAINTED_SCALAR) 
    tainted_data: Passing tainted variable > s.size() + 1UL to a tainted sink. 

Я понимаю, что я не должен доверять какие-либо данные, считанные из файла, но я не могу понять, как проверить данные на данном этапе. Я уже проверяю, что s.size() находится в пределах правдоподобного (хотя и довольно большого) диапазона в if -clause над ошибочной линией.

Так почему же обложка бросает предупреждение на меня?

Кроме того, какие другие стратегии для проверки ввода должны применяться?

+3

На самом деле не стоит называть 'eof()'. http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong – Galik

+2

Вместо проверки 's.empty()' лучше проверить, удалось ли прочитать: 'while (is >> s) {/ * чтение s преуспело здесь */...}' – Galik

+2

Возможно, анализатор кода не справляется с пониманием того, что 's.size()' вернет одинаковое значение для всех трех вызовов. Вы пытались присвоить возвращаемое значение переменной и использовать ее? – 5gon12eder

ответ

2

В следующем разделе

if (s.empty()) 
    continue; 
if (s.size() < 1 || s.size() > 0x7FFFFFFF) 
    { 
    std::cout << "implausible data" << std::endl; 
    continue; 
    } 
char * ss = new char[s.size() + 1]; 

логика проверки опирается на тот важный факт, что s.size() будет возвращать то же значение каждый раз, когда он вызывается. Хотя в этом случае мы (люди) знаем, что это будет верно, статический анализатор кода может не справиться с этим.

В качестве обходного пути попробуйте ввести локальную переменную и использовать ее.

const std::size_t length = s.size(); 
if (!length) 
    continue; 
if (length < 1 || length > 0x7FFFFFFF) 
    { 
    std::cout << "implausible data" << std::endl; 
    continue; 
    } 
char * ss = new char[length + 1]; 

Здесь просто для анализатора сказать, что length не изменит свое значение.

Независимо от того, стоит ли такое кодирование вокруг ограничений инструментов статического анализатора, открыто для обсуждения. GNU Coding Standards обескуражить его.

Don’t make the program ugly just to placate static analysis tools such as lint, clang, and GCC with extra warnings options such as -Wconversion and -Wundef . These tools can help find bugs and unclear code, but they can also generate so many false alarms that it hurts readability to silence them with unnecessary casts, wrappers, and other complications. For example, please don’t insert casts to void or calls to do-nothing functions merely to pacify a lint checker.

Лично я не чувствую себя слишком плохо о нем до тех пор, как читаемость кода не страдает слишком много. В угловых случаях добавление комментария, объясняющего, почему все делается так, как они сделаны, может быть хорошей идеей.

+0

дополнительные пункты для упоминания * Стандарты кодирования GNU *! –