Forem: Alex Astva

How to shoot yourself in the foot in C and C++. Haiku OS Cookbook

Alex Astva — Fri, 26 Jul 2019 07:09:48 +0000

This story goes back to 2015, when Haiku OS and PVS-Studio static analyzer developers decided to join forces and improve this OS code quality. At first it was more like an experiment, as there was no Linux analyzer at that time and the team had to work only with the compiled executable analyzer file. The entire infrastructure for parsing compiler parameters, running preprocessor, analysis paralleling and so on was taken from the Compiler Monitoring UI utility in C#, which was ported in parts to the Mono platform in order to be run in Linux.

Now the Haiku project is built using the cross compiler under various OSs, except Windows. Once again, I'd like to mention the convenience and documentation completeness related to Haiku OS building and thank Haiku OS developers for their help in building the project.

Interestingly, the nature of programming errors is such that they don't disappear if you don't search for them and don't pay attention to the code quality. Haiku developers tried to use Coverity Scan, but, most sadly, the last analysis run was almost two years ago. Even though the analysis has been configured in 2014 using Coverity, it didn't stop us from writing two long articles on errors review in 2015 (part 1, part 2). Now four years later, a new article about checking this project appears.

Note:here will be some interesting errors from the project, a more complete report can be checked out in the article "How to shoot yourself in the foot in C and C++. Haiku OS Cookbook".

So let's move on to the errors:

Formal security

V597 The compiler could delete the 'memset' function call, which is used to flush 'f_key' object. The memset_s() function should be used to erase the private data. dst_api.c 1018

#ifndef SAFE_FREE
#define SAFE_FREE(a) \
do{if(a != NULL){memset(a,0, sizeof(*a)); free(a); a=NULL;}} while (0)
....
#endif

DST_KEY *
dst_free_key(DST_KEY *f_key)
{
  if (f_key == NULL)
    return (f_key);
  if (f_key->dk_func && f_key->dk_func->destroy)
    f_key->dk_KEY_struct =
      f_key->dk_func->destroy(f_key->dk_KEY_struct);
  else {
    EREPORT(("dst_free_key(): Unknown key alg %d\n",
       f_key->dk_alg));
  }
  if (f_key->dk_KEY_struct) {
    free(f_key->dk_KEY_struct);
    f_key->dk_KEY_struct = NULL;
  }
  if (f_key->dk_key_name)
    SAFE_FREE(f_key->dk_key_name);
  SAFE_FREE(f_key);
  return (NULL);
}

The analyzer has detected suspicious code, meant for secure private data clearing. Unfortunately, the SAFE_FREE*macro which expands into the *memset, free*calls and *NULL*assignment doesn't make the code safer, as it's all removed by the compiler right when optimizing with *O2.

By the way, it is nothing else but CWE-14: Compiler Removal of Code to Clear Buffers.

Miscellaneous

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 101

static void
dump_acpi_namespace(acpi_ns_device_info *device, char *root, int indenting)
{
  char output[320];
  char tabs[255] = "";
  ....
  strlcat(tabs, "|--- ", sizeof(tabs));
  ....
  while (....) {
    uint32 type = device->acpi->get_object_type(result);
    snprintf(output, sizeof(output), "%s%s", tabs, result + depth);
    switch(type) {
      case ACPI_TYPE_INTEGER:
        strncat(output, "     INTEGER", sizeof(output));
        break;
      case ACPI_TYPE_STRING:
        strncat(output, "     STRING", sizeof(output));
        break;
      ....
    }
    ....
  }
  ....
}

The difference between *strlcat*and *strncat*functions is not very obvious for someone who is unfamiliar with the description of these functions. The *strlcat*function expects the size of the entire buffer as the third argument while the *strncat*function - the size of the free space in a buffer, which requires evaluating a needed value before calling the function. But developers often forget or don't know about it. Passing the entire buffer size to the *strncat*function can lead to buffer overflow, as the function will consider this value as an acceptable number of characters to copy. The *strlcat*function doesn't have such a problem. But you have to pass strings, ending with terminal null so that it worked properly.

Errors with the free function

V575 The null pointer is passed into 'free' function. Inspect the first argument. PackageFileHeapWriter.cpp 166

void* _GetBuffer()
{
  ....
  void* buffer = malloc(fBufferSize);
  if (buffer == NULL && !fBuffers.AddItem(buffer)) {
    free(buffer);
    throw std::bad_alloc();
  }
  return buffer;
}

Someone made an error here. The ||operator has to be used instead of &&. Only in this case the std::bad_alloc()*exception will be thrown in case if memory allocation (using the *malloc function) failed.

Errors with the delete operator

V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] fOutBuffer;'. Check lines: 26, 45. PCL6Rasterizer.h 26

class PCL6Rasterizer : public Rasterizer
{
public:
  ....
  ~PCL6Rasterizer()
  {
    delete fOutBuffer;
    fOutBuffer = NULL;
  }
  ....
  virtual void InitializeBuffer()
  {
    fOutBuffer = new uchar[fOutBufferSize];
  }
private:
  uchar* fOutBuffer;
  int    fOutBufferSize;
};

It's a common error to use the delete*operator instead of *delete[]. It is easiest to make a mistake when writing a class, as the destructor's code is often far from the memory locations. Here, the programmer incorrectly frees the memory stored by the *fOutBuffer*pointer in the destructor.

Follow our PVS-Studio team blog to see another Haiku OS errors review coming out soon for those who read the first part up to the end. The full analyzer report will be sent to developers before posting this errors review, so some errors might be fixed by the time you're reading this. To pass the time between the articles, I suggest downloading and trying PVS-Studio for your project.

Every 4000 lines of Android code contain a potential vulnerability

Alex Astva — Fri, 03 Aug 2018 13:56:09 +0000

Last year, developers of the static code analyzer PVS-Studio already cited the result of their research on the operating system Tizen example and now again the choice fell on another, no less popular OS, Android. Looking ahead, I want to note that I didn't expect such a huge number of mistakes in such a popular product!

In the article, mistakes carried in even such a high-quality codeare striped on examples, in fact even those tested by Coverity.

In total, the developers described about 490 potential vulnerabilities. For example, CWE-14: Compiler Removal of Code to Clear Buffers, which is a serious type of potential vulnerability and leads to access to private data.

Among the others you can find:
• CWE-119: Improper Restriction of Operations within the Bounds of a Memory Buffer
• CWE-393: Return of Wrong Status Code
• CWE-480: Use of Incorrect Operator
• CWE-561: Dead Code
• CWE-690: Unchecked Return Value to NULL Pointer Dereference
• CWE-762: Mismatched Memory Management Routines

And many other interesting examples of almost 2 million lines of code in C and C ++! Wow!

Source - https://www.viva64.com/en/b/0579/

Amazon Lumberyard: A Scream of Anguish

Alex Astva — Mon, 09 Jul 2018 14:33:17 +0000

Video games are among the most popular software products. Now a new game engine, Amazon Lumberyard, has joined this huge industry. The project is currently in the beta phase and still has time to get rid of the bugs and improve. Its authors have a lot of work to do to make sure they don't disappoint millions of gamers and game developers in the nearest future.

Amazon Lumberyard is a free cross-platform triple-A game engine developed by Amazon and based on the architecture of CryEngine, which was licensed from Crytek in 2015. I already checked CryEngine twice, in august 2016 and april 2017, and I'm sorry to say it, but the code quality had decreased since the first check. I was wondering the other day how Amazon had used the engine and took a look at the new product. I must admit they did make a great environment. The user documentation and environment deployment software are really awesome too. But the code is messed up again! I hope Amazon can afford to allocate much more resources for that project and will start caring about code quality at last. By writing this review I hope to draw their attention to this problem and persuade them to take a new approach to the development process.

The source files under analysis refer to the latest Amazon Lumberyard version, 1.14.0.1, and were downloaded from the Github repository. Star Citizen is one of the first games to be built on the Lumberyard engine. If you are looking to play it, then welcome to take a peek under the hood with me.

Couple of words about development of a new static analyzer for Java code

Alex Astva — Fri, 22 Jun 2018 09:00:30 +0000

PVS-Studio static analyzer team, which until recently was searching for bugs and potential vulnerabilities only in C, C++, and C# code, has prepared a new version of their tool for Java code as well. Despite the fact that in the Java world there is already a number of static analysis tools, developers believe that their analyzer can be powerful and will be a good competition.

One of developers wrote in his article about the way how PVS-Studio for Java was created. First of all, it was necessary to figure out the development process of a syntax tree and semantic model. As these elements are basic, static analyzer is built around them. In addition to them, the analyzer also requires data flow analysis, which enables you to calculate the possible values of variables and expressions in each point of the program and, thanks to that, find errors. Also the analyzer needs the annotation mechanism, diagnostic rules, integration, testing, and other techniques, explained in detail in the article.

Traditionally, the author gave examples of errors from different open source projects, that the Java analyzer has detected. The author also noted that in the future articles with a more detailed report on each project will be available. So far you can review errors from the Hibernate, Hive, JavaParser, Jenkins projects and several others.

Besides that, when the first alpha version of the Java analyzer is available, developers suggest participating in its testing for those who would like to. To do this, write to their support and they will contact you.

Full article - https://www.viva64.com/en/b/0572/

Static Analysis in Video Game Development: Top 10 Software Bugs

Alex Astva — Fri, 04 May 2018 13:32:44 +0000

If you are a software developer working in the video game industry and wondering what else you could do to improve the quality of your product or make the development process easier and you don’t use static analysis – it’s just the right time to start doing so. You doubt that? OK, I’ll try to convince you. And if you are just looking to see what coding mistakes are common with video-game and game-engine developers, then you’re, again, at the right place: I have picked the most interesting ones for you.

One of the best ways to prove that static analysis is a useful method is probably through examples showing it in action. That’s what the PVS-Studio team does while checking open-source projects. After publishing of various articles, we compiled our top-10 mistakes from the program code in the video-game industry, and now we suggest you read this article - https://www.viva64.com/en/b/0570/

List of projects considered in the article:

X-Ray Engine
CryEngine V
Space Engineers
Quake III Arena GPL
Unity
Godot
Doom 3
Xenko -Unreal Engine 4

Although video-game development includes a lot of steps, coding remains one of the basic ones. Even if you don’t write thousands of code lines, you have to use various tools whose quality determines how comfortable the process is and what the ultimate result will be. Static analysis is a very useful tool when developing, and one more option to help you improve the quality of your code (and thus of the final product).

Static Code Analyzer: the Additional Insurance of the Medical Software

Alex Astva — Wed, 21 Mar 2018 08:24:47 +0000

Software bugs can lead not only to material losses, but also can damage human's health. For example, actors on the stage of a theatre can get injured if suddenly one of the scenery begins to go down on the stage at the wrong time. However, the connection between the errors in code and the health damage of medical software is more obvious. Let's talk about this topic.

This article focuses on the teams of developers who create the programs for a medical equipment. I hope they will not stay indifferent and will check their code. Let's recall two famous cases where errors in programs, related to medicine, became the reason for bad news.

Firstly, it is a series of tragic events caused by the errors in the Therac-25 device of radiation therapy. This device has caused at least six overdoses of radiation within the period from June 1985 to January 1987, some patients received doses of tens of thousands of rad. At least two people died directly from the radiation overdoses. Software bugs of the device were the reason of the tragedies and the main problem was the incorrect security strategy.

Secondly, the software bugs can also cause harm indirectly. For example, the bugs in the software for MRI-scanners raise the questions about the 40 000 researches. For several decades, neuroscientists and cognitive psychologists has used statistical programs AFNI, SPM and FSL to analyze fMRI data. As it turned out, because of the incorrect algorithms, these programs might return up to 70% of false positive results instead of the projected 5%.

As you can see, the code errors can lead not only to troubles, such as a crash or a loss of data, but also to much more serious consequences, which influence the life and the health of many people throughout many years.

Moreover, the developer is responsible not only for his own code, but for the code of the used libraries. This situation is completely real, when the artifacts appear when creating an image/video due to an error from a third-party library and this will lead to confusion when diagnosing.

This is not an abstract theoretical problem. I myself faced a situation, in which when porting programs to 64-bit system an error causing incorrect handling of MRI data began to reveal itself. Fortunately, the error showed itself very clearly: a large fragment of the image was absent. However, the error might not be that noticeable and consist in the incorrect displaying of some details and it'll be much harder to detect it.

More information about this error is available in the article "PVS-Studio project - 10 years of failures and successes". It is this and some other 64-bit errors that inspired the creation of the Viva64 tool, which then turned into a PVS-Studio static code analyzer.

It is impossible to predict where the errors can be and what errors can lead to trouble. The error can be complex and not necessarily spoil life, lurking in the algorithm of data processing and displaying. I can imagine a situation where because of an error in the comparison function, data of the wrong patient will be selected for processing, or the program, describing the condition of the patient, will not notice some differences in the data structure.

I invite all readers to start using the PVS-Studio static code analyzer. Yes, the analyzer, like any other tool, does not guarantee the absence of errors in your programs. However, it becomes an additional line of defense on the field of the battle against the bugs. It can help detect a lot of errors at the early stages of development and may help to save someone's health.