Skip to main content

Inappropriate coding practices

Need

Enforce proper encapsulation in coding practices

Context

  • Usage of C# for developing robust and scalable applications

Description

Non compliant code

public class User
{
public string Name;
public string Email;
public string Password;
}

In the above code snippet, we have a User class with three fields: Name, Email, and Password. All these fields are declared as public.

This is a violation of the encapsulation principle, one of the four fundamental principles of Object Oriented Programming (OOP). Encapsulation is the mechanism of hiding the data (variables or state) and the methods working on the data (behaviors or code). It also leads to data security - data hiding.

By declaring these fields as public, they can be accessed and modified directly from any part of the application. This could potentially lead to unintended changes to these fields, causing unexpected behavior in the application.

For instance, consider the Password field. If it's public, any part of the application could potentially change a user's password without going through a proper password change procedure (like verifying the old password, sending confirmation emails, etc.). This could lead to security issues.

Moreover, by making fields public, we are exposing the internal implementation of our class. If we decide to change how we store the data in the future (e.g., changing Name from a string to a Name class), we would need to change all parts of the application that directly access the Name field.

In summary, making class fields public can lead to potential security issues, unexpected behavior, and makes our code less flexible to changes.

Steps

  • Encapsulate the fields by making them private.
  • Provide public getter and setter methods for accessing and modifying the fields.
  • Apply access modifiers to the getter and setter methods to control the visibility and mutability of the fields.
  • Update the code that uses the User class to use the getter and setter methods instead of directly accessing the fields.

Compliant code

public class User
{
private string _name;
private string _email;
private string _password;

public string Name
{
get { return _name; }
set { _name = value; }
}

public string Email
{
get { return _email; }
set { _email = value; }
}

public string Password
{
get { return _password; }
set { _password = value; }
}
}

The original code had a vulnerability due to inappropriate coding practices. The fields of the User class were defined as public, which does not respect the encapsulation principle of object-oriented programming. This could potentially allow unauthorized access and modification of the data.

The fixed code above addresses this vulnerability by encapsulating the fields. The fields Name, Email, and Password are now private, which means they can only be accessed and modified within the User class.

To allow controlled access and modification of these fields, public getter and setter methods are provided. The get method returns the value of the field, and the set method updates the value of the field. This way, the fields can still be accessed and modified, but in a controlled and secure manner.

Any code that uses the User class should now use these getter and setter methods instead of directly accessing the fields. This ensures that the encapsulation principle is respected, enhancing the security and integrity of the data.

References