"C++ Code Reviews"
Author: Amit Kumar Sharma
C++ has been an efficient programming language in years. Its area of coverage was in systems software, embedded software, device drivers, compliers, server applications and most significantly game programming. C++ is the foremost chosen language whenever hardware programming and design is considered. C++ has been an efficient and works closely with the os and direct memory access.
With its capability to access memory makes in inherently one of the most unsafe language as well.
This introduces much different kind of security bugs as well and makes it more susceptible to different kind of vulnerabilities. Though security issues are also prevalent in many other programming languages, thus it is advised to consider security as a major chunk in your development process.
To attain a higher quality if system which is more resilient to attacks one should follow a secure coding standard. These coding standards allow/encourage the programmers to follow a set of rules keeping in mind security.
There can be three types of code review.
Automatic or tool based:
The automatic process involves a using of a tool commercialized or freeware where in the code base is fed and the tool gives the result post which the result are analyzed for false positives. The manual code review process involves going through the code and finding security bugs. This is a very time-consuming process and it involves a highly skilled person, and thus also increases the cost.
The hybrid process in turn involves both of the above. While the code is run on a tool, the security expert goes through the code manually hunting down bugs if any. It is suggested for going with the hybrid style of code review as the process involves less time and is also efficient for large code bases.
Apart from the other process involved reviewing the code manually for security defects is the most important and effective procedure to make your code hack resilient. This requires a highly skilled person. We will discuss on how to perform a manual code review for any C++ code base. We will to go in deep with the vulnerabilities in the wild, instead we will see on how a code can be reviewed for security bugs,
This process is also called as static code review.
Once you are ready with the code, as always said by the great old people you can come out with a plan which makes the work easier and more traceable. People technically called it as a threat model. Remember there is no problem ever to make your own threat model. Does the code run by default?
As a first step, one can ask the architect or the developer on what the code does before starting the review. This phase is very similar to a reconnaissance phase of any penetration test where in you gather enough information about your target.
The following questions can be helpful:
What is the environment in which the code is going to be deployed?
Is there any kind of user roles defined?
Does the code involve any network interaction?
Does the code run with elevated privileges?
Does the code have any kind of previous vulnerability which was fixed?
What is the data that will be handled by the code and what is the sensitivity of the data handled?
Any third party integrations in the code?
How much reusable the code is?
Note: these are just sample question. You can always have your own set of questions.
The only purpose of the questions is to become familiar with the code base and understand the flow of the code so that you can analyze the code better.
The next step can be frisking the code once and identifying the places where more in-depth analysis is required. This step also makes you aware of the flow of the code and also cross checking the answers you got for your first set of questions. For example different kind of function calls, class inheritance etc.
Places where you can hook yourself up for making pointers are:
Usage of inbuilt functions
System calls checks
These set flags are then analyzed to search for security bugs. Search for the input variables or where there is a user intervention and check whether they are properly sanitized for the kind of input it is accepting. Proper access to the uses of these variables is supposed to be checked. We can next check for the access management scheme and its implementation of the different roles present in the application. Always look for keyword in the code like password and its look alike, registry keys, crypto etc. with different kind of logical operators used in the code along with all the different kind of words used with sensitive kind of data.
It is always advisable to use an already developed crypto algorithm than making your own. Reviewing the proper usage of the inbuilt function is also very important as it can also pose a serious security threat to the application. For eg using the function rand() or random() for randomly generation of number should be use appropriately for not getting exploited by it. One should also review the access of the files and its permission on who can write to the file and who can read the file. Proper usage of function calls for files and/or system files should be checked. Evaluating files once opened should be from proper file handler or validator rather than by any other resource. Proper usage of the functions like access() should be checked for its access privileges. Follow the rule of least privileges to avoid any kind of privilege escalation. Avoid running your program in root/administrative privilege.
One should also be aware of the known vulnerabilities of the language for the code you are reviewing. In case of C++ we can always consider vulnerabilities like buffer overflow; integer overflow C++ is more prone to buffer overflow kind of vulnerabilities. These are severe kind of vulnerabilities as the attacker can inject code of their choice into the running process and can control your process and system in turn.
The compile error sometimes gives a lot of information about security bugs as well. So analyzing those can also be a part of your critical review. Declaring objects as immutable can avoid flaws of state change. The objects are not supposed to change state post declaration.
Checking the proper flow of the code relation wise is also an advisable step for your review. For eg we should always concentrate on different kind of variable call, function calls and inheritance of classes in order to avoid any kind of mishap calls between them thereby introducing flaws.
It is always helpful to do a peer code review as sometimes it becomes very difficult to identify security threats with one perspective in mind. Always cross check with your peer to identify new vulnerabilities if any.
The final and most difficult step is documenting your findings (i am sure it is most difficult for all of you). But the main point is the report says your work. Always document all your findings and errors stating your approach and remediation plan and risk per flaw.
Some function which should be avoided:
System(), popen(),execlp(),execvp() wsystem(), the ShellExecute() family of function strcpy(), strcat(), sprintf(), vsprintf(), gets() scanf(), scanf(), fscanf(), sscanf(), vscanf(), vsscanf(), and vfscanf() ealpath(3), getopt(3), getpass(3), streadd(3), strecpy(3), and strtrns(3)
About the Author:
Amit Kumar Sharma commonly known as AKS-44 has an engineering degree in Electronics & Communication and works in Information Security for a reputed firm. He is passionate about Security and spends his time learning/researching in the wild.