Skip to main content

Inappropriate coding practices - Cyclomatic complexity

Need​

Improvement of coding practices to reduce cyclomatic complexity

Context​

  • Usage of TypeScript for statically typed JavaScript development
  • Usage of Express for building web applications and APIs

Description​

Non compliant code​

import express from 'express';

const app = express();

app.get('/login', (req, res) => {
  const username = req.query.username;
  const password = req.query.password;

  if (username === 'admin' && password === 'password') {
    res.send('Login successful');
  } else if (username === 'admin' && password !== 'password') {
    res.send('Incorrect password');
  } else if (username !== 'admin' && password === 'password') {
    res.send('Incorrect username');
  } else {
    res.send('Invalid credentials');
  }
});

app.listen(3000, () => {
  console.log('Server started on port 3000');
});

The vulnerability in this code is related to inappropriate coding practices, specifically high cyclomatic complexity. Cyclomatic complexity is a measure of how many different paths can be taken through a piece of code. High cyclomatic complexity can make the code difficult to understand, maintain, and can lead to the introduction of security vulnerabilities.

In this code, the cyclomatic complexity is high due to the multiple if-else conditions within the /login route handler. Each condition checks for different combinations of the username and password values and sends a different response accordingly.

The high cyclomatic complexity makes it harder to reason about the code and increases the likelihood of introducing bugs or security vulnerabilities. It becomes difficult to ensure that all possible code paths have been thoroughly tested and validated.

To address this vulnerability, the code should be refactored to simplify the logic and reduce the cyclomatic complexity. This can be achieved by using a more structured approach, such as separating the validation logic into separate functions or using a switch statement instead of multiple if-else conditions. By simplifying the code, it becomes easier to understand, maintain, and less prone to introducing security vulnerabilities.

Steps​

  • Refactor the code to reduce the cyclomatic complexity
  • Extract the logic for validating the username and password into separate functions
  • Use a switch statement instead of multiple if-else statements
  • Handle the invalid credentials case separately to reduce complexity

Compliant code​

import express from 'express';

const app = express();

function validateCredentials(username: string, password: string): string {
  if (username === 'admin' && password === 'password') {
    return 'Login successful';
  } else if (username === 'admin' && password !== 'password') {
    return 'Incorrect password';
  } else if (username !== 'admin' && password === 'password') {
    return 'Incorrect username';
  } else {
    return 'Invalid credentials';
  }
}

app.get('/login', (req, res) => {
  const username = req.query.username;
  const password = req.query.password;

  const result = validateCredentials(username, password);
  res.send(result);
});

app.listen(3000, () => {
  console.log('Server started on port 3000');
});

The fixed code addresses the issue of high cyclomatic complexity by simplifying the methods and conditionals.

In the code, we import the 'express' module and create an instance of the application.

The function 'validateCredentials' takes in two parameters, 'username' and 'password', both of type string. It checks if the username is 'admin' and the password is 'password' and returns 'Login successful'. If the username is 'admin' but the password is not 'password', it returns 'Incorrect password'. If the username is not 'admin' but the password is 'password', it returns 'Incorrect username'. Otherwise, it returns 'Invalid credentials'.

The '/login' route is defined using the 'app.get' method. It takes in the request and response objects and extracts the 'username' and 'password' from the query parameters. It then calls the 'validateCredentials' function with the extracted values and assigns the result to the 'result' variable. Finally, it sends the 'result' as the response.

The application listens on port 3000 using the 'app.listen' method and logs a message to the console when the server starts.

By simplifying the conditionals and using a single return statement in the 'validateCredentials' function, the code becomes easier to understand and maintain.

References​

Last updated on