-
Notifications
You must be signed in to change notification settings - Fork 33
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
9ec852f
commit 34ba78c
Showing
15 changed files
with
531 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
name: SonarCloud | ||
on: | ||
push: | ||
branches: | ||
- main | ||
pull_request: | ||
types: [opened, synchronize, reopened] | ||
jobs: | ||
build: | ||
name: Build and analyze | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v3 | ||
with: | ||
fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis | ||
- name: Set up JDK 11 | ||
uses: actions/setup-java@v3 | ||
with: | ||
java-version: 17 | ||
distribution: 'zulu' # Alternative distribution options are available. | ||
- name: Cache SonarCloud packages | ||
uses: actions/cache@v3 | ||
with: | ||
path: ~/.sonar/cache | ||
key: ${{ runner.os }}-sonar | ||
restore-keys: ${{ runner.os }}-sonar | ||
- name: Cache Maven packages | ||
uses: actions/cache@v3 | ||
with: | ||
path: ~/.m2 | ||
key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }} | ||
restore-keys: ${{ runner.os }}-m2 | ||
- name: Build and analyze | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any | ||
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} | ||
run: mvn -B verify org.sonarsource.scanner.maven:sonar-maven-plugin:sonar -Dsonar.projectKey=SonarSourceResearch_DeepSAST_Demo |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,69 @@ | ||
# deeper-sast-demo | ||
# deeper-sast-demo | ||
|
||
## Storyboard | ||
|
||
The goal of this demo is to show the DeepSAST capabilities of the Java engine. We want to highlight that the usage of external libraries can introduce severe security vulnerabilities, which can be detected by Deep SAST. | ||
|
||
The demo is a fictive Spring application implementing different functionalities which are vulnerable to security issues detected by our engine. All of these issues contain at least one step where the data flow: | ||
|
||
1. originates from a user-controllable source within a library, | ||
2. passes through a library, or | ||
3. ends in a dangerous sink within a library. | ||
|
||
Thus these issues are only detected because of the DeepSAST feature of the engine. | ||
|
||
There are four issues: two of these are already committed to the main branch of the application. Additionally, there are two pending pull requests (PR), which each introduce another vulnerability. For these issues, the chosen examples aim to demonstrate that the proposed source code in the PR does not look dangerous or security-sensitive and would likely be merged. | ||
|
||
### Issue 1 - Session Cookie Handling (main branch) | ||
|
||
* Vulnerability Type: Deserialization ([S5135](https://rules.sonarsource.com/java/RSPEC-5135/)) | ||
* DeepSAST Dataflow: | ||
* Passthrough: `org.apache.commons.codec.binary.Base64.decodeBase64` | ||
|
||
This vulnerability resides within the session cookie handling of the application. A vulnerability is introduced by deserializing user-controllable data from a header (`Session-Auth`), which can be exploited to execute arbitrary code. The data provided in the header is passed through the `decodeBase64` library function before being deserialized. | ||
|
||
|
||
### Issue 2 - User Images (main branch) | ||
|
||
* Vulnerability Type: Path Injection ([S2083](https://rules.sonarsource.com/java/RSPEC-2083/)) | ||
* DeepSAST Dataflow: | ||
* Source: org.springframework.web.context.request.getRemoteUser | ||
* Passthrough: org.apache.tomcat.util.buf.UDecoder.URLDecode | ||
* Sink: cn.hutool.cache.file.LRUFileCache.getFileBytes | ||
|
||
This vulnerability resides within the code responsible for retrieving user images. The library function `getRemoteUser` is used to retrieve the user-controllable username, which is passed through the `URLDecode` library function. The result is concatenated to a file path, which is passed to the `getFileBytes` library function introducing a path injection vulnerability. | ||
|
||
|
||
### Issue 3 - User Migration (PR 1 - Introduce user migration feature) | ||
|
||
* Vulnerability Type: SQL Injection ([S3649](https://rules.sonarsource.com/java/RSPEC-3649/)) | ||
* DeepSAST Dataflow: | ||
* Sink: com.mysql.cj.jdbc.ConnectionImpl.setSavepoint | ||
|
||
This PR adds a feature to migrate users from the existing H2 database to MySQL. Although the proposed change does not seem to contain any vulnerabilities, the `setSavepoint` library function is vulnerable to SQL injection if the passed argument is user-controllable. Thus this PR introduces a critical vulnerability due to the usage of the unsafe library function. | ||
|
||
### Issue 4 - XML User Import (PR 2 - Allow the import of users) | ||
|
||
* Vulnerability Type: Deserialization ([S5135](https://rules.sonarsource.com/java/RSPEC-5135/)) | ||
* DeepSAST Dataflow: | ||
* Sink: ca.odell.glazedlists.impl.io.BeanXMLByteCoder.decode | ||
|
||
This PR adds a new feature to import users from an XML file. Although the code itself does not seem to contain any vulnerabilities, the `decode` library function is vulnerable to deserialzation if the passed argument is user-controllable. Thus this PR introduces a critical vulnerability due to the usage of the unsafe library function. | ||
|
||
|
||
## Setup instructions | ||
|
||
This repository is supposed to be added as a SonarCloud project for analysis via GitHub actions. | ||
|
||
* Fork this project *with all branches* (untick the default checkbox, "Copy the `main`` branch only"). | ||
* Go to [sonarcloud.io](https://sonarcloud.io/sessions/new) and sign up with your GitHub account. | ||
* Create a new organization under your name if there is none. | ||
* Give SonarCloud permission to see the forked repository. | ||
* Add your repository as a new Project. | ||
* Go to `Administration` -> `Analysis Method` and uncheck `Automatic Analysis`. | ||
* Select `Set up analysis via other methods` -> `With GitHub Actions`. | ||
* Add the displayed GitHub Secret to your repository. | ||
* The `pom.xml` does not need to be adjusted. | ||
* Update the `.github/workflows/build.yml` file in the main branch with the displayed content and ensure that the `java-version` is set to `17`. | ||
|
||
The first two issues will be displayed on the `main` branch and the other two issues on distinct Pull Requests. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
<modelVersion>4.0.0</modelVersion> | ||
<parent> | ||
<groupId>org.springframework.boot</groupId> | ||
<artifactId>spring-boot-starter-parent</artifactId> | ||
<version>3.1.1</version> | ||
<relativePath/> <!-- lookup parent from repository --> | ||
</parent> | ||
<groupId>com.example</groupId> | ||
<artifactId>deeper-sast-demo</artifactId> | ||
<version>0.0.1-SNAPSHOT</version> | ||
<name>DashboardManager</name> | ||
<description>deeper-sast-demo</description> | ||
<properties> | ||
<java.version>17</java.version> | ||
<sonar.organization>sonarsourceresearch</sonar.organization> | ||
<sonar.host.url>https://sonarcloud.io</sonar.host.url> | ||
</properties> | ||
<dependencies> | ||
<dependency> | ||
<groupId>org.springframework.boot</groupId> | ||
<artifactId>spring-boot-starter-web</artifactId> | ||
</dependency> | ||
|
||
<dependency> | ||
<groupId>com.h2database</groupId> | ||
<artifactId>h2</artifactId> | ||
<scope>runtime</scope> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.springframework.boot</groupId> | ||
<artifactId>spring-boot-starter-test</artifactId> | ||
<scope>test</scope> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.springframework.boot</groupId> | ||
<artifactId>spring-boot-starter-data-jpa</artifactId> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.springframework.security</groupId> | ||
<artifactId>spring-security-crypto</artifactId> | ||
<version>6.0.2</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.projectlombok</groupId> | ||
<artifactId>lombok</artifactId> | ||
</dependency> | ||
<dependency> | ||
<groupId>commons-lang</groupId> | ||
<artifactId>commons-lang</artifactId> | ||
<version>2.6</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>commons-codec</groupId> | ||
<artifactId>commons-codec</artifactId> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.apache.tomcat</groupId> | ||
<artifactId>tomcat-util</artifactId> | ||
<version>10.1.7</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>com.github.oshi</groupId> | ||
<artifactId>oshi-core</artifactId> | ||
<version>6.4.0</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>com.servlets</groupId> | ||
<artifactId>cos</artifactId> | ||
<version>09May2002</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>javax.servlet</groupId> | ||
<artifactId>javax.servlet-api</artifactId> | ||
<version>4.0.1</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>cn.hutool</groupId> | ||
<artifactId>hutool-all</artifactId> | ||
<version>5.8.18</version> | ||
</dependency> | ||
</dependencies> | ||
|
||
<build> | ||
<plugins> | ||
<plugin> | ||
<groupId>org.springframework.boot</groupId> | ||
<artifactId>spring-boot-maven-plugin</artifactId> | ||
</plugin> | ||
</plugins> | ||
</build> | ||
|
||
</project> |
11 changes: 11 additions & 0 deletions
11
src/main/java/com/dashboardmanager/DashboardManagerApplication.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package com.dashboardmanager; | ||
|
||
import org.springframework.boot.SpringApplication; | ||
import org.springframework.boot.autoconfigure.SpringBootApplication; | ||
|
||
@SpringBootApplication | ||
public class DashboardManagerApplication { | ||
public static void main(String[] args) { | ||
SpringApplication.run(DashboardManagerApplication.class, args); | ||
} | ||
} |
148 changes: 148 additions & 0 deletions
148
src/main/java/com/dashboardmanager/controller/UserController.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
package com.dashboardmanager.controller; | ||
|
||
import cn.hutool.cache.file.LRUFileCache; | ||
import com.dashboardmanager.model.Session; | ||
import com.dashboardmanager.model.User; | ||
import com.dashboardmanager.repository.SessionsRepository; | ||
import com.dashboardmanager.repository.UsersRepository; | ||
import com.dashboardmanager.utils.FileUtils; | ||
import com.dashboardmanager.utils.SessionHeader; | ||
import jakarta.servlet.http.HttpServletRequest; | ||
import jakarta.servlet.http.HttpServletResponse; | ||
import org.apache.commons.lang.RandomStringUtils; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.core.io.ByteArrayResource; | ||
import org.springframework.core.io.Resource; | ||
import org.springframework.http.HttpStatus; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.PostMapping; | ||
import org.springframework.web.bind.annotation.RequestParam; | ||
import org.springframework.web.bind.annotation.RestController; | ||
import org.springframework.web.context.request.ServletWebRequest; | ||
import org.springframework.web.servlet.view.RedirectView; | ||
|
||
import java.io.*; | ||
import java.security.SecureRandom; | ||
|
||
import static org.apache.commons.codec.binary.Base64.decodeBase64; | ||
|
||
@RestController | ||
public class UserController { | ||
|
||
@Autowired | ||
private UsersRepository usersRepository; | ||
|
||
@Autowired | ||
private SessionsRepository sessionsRepository; | ||
|
||
private LRUFileCache fileCache = new LRUFileCache(100); | ||
|
||
@GetMapping("/") | ||
public RedirectView index(HttpServletRequest request) { | ||
if (isAuthenticated(request)) { | ||
return new RedirectView("dashboard"); | ||
} | ||
else { | ||
return new RedirectView("login"); | ||
} | ||
} | ||
|
||
@PostMapping("/login") | ||
public String login(HttpServletResponse response, @RequestParam(value = "username") String username, @RequestParam(value = "password") String password) { | ||
User user = usersRepository.findByName(username); | ||
BCryptPasswordEncoder passwordEncoder = new BCryptPasswordEncoder(); | ||
if (passwordEncoder.matches(password, user.getPassword())) { | ||
Session session = new Session(); | ||
session.setUser(user); | ||
session.setSessionId(RandomStringUtils.random(20, 0, 0, true, true, null, new SecureRandom())); | ||
sessionsRepository.save(session); | ||
response.addHeader("Session-Auth", createSessionHeader(session)); | ||
return "login successful"; | ||
} | ||
else { | ||
return "invalid credentials!"; | ||
} | ||
} | ||
|
||
@GetMapping("/dashboard") | ||
public String dashboard(HttpServletRequest request) { | ||
if (isAuthenticated(request)) { | ||
User user = getUser(request); | ||
if (user == null) { | ||
return "an error occurred!"; | ||
} else { | ||
return String.format("Welcome %s!", user.getName()); | ||
} | ||
} else { | ||
return "forbidden!"; | ||
} | ||
} | ||
|
||
@GetMapping("/user/images") | ||
public ResponseEntity<Resource> getUserImage(ServletWebRequest request, @RequestParam(value = "image_quality") String imageQuality) { | ||
try { | ||
String extension = ""; | ||
switch (imageQuality) { | ||
case "scalable": | ||
extension = ".svg"; | ||
break; | ||
case "high": | ||
extension = ".png"; | ||
break; | ||
case "low": | ||
extension = ".jpg"; | ||
break; | ||
} | ||
String imageFile = FileUtils.getInstance().getUserImagePath(request.getRemoteUser()) + extension; | ||
final ByteArrayResource inputStream = new ByteArrayResource(fileCache.getFileBytes(imageFile)); | ||
return ResponseEntity.status(HttpStatus.OK).contentLength(inputStream.contentLength()).body(inputStream); | ||
} catch (Exception e) { | ||
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).build(); | ||
} | ||
} | ||
|
||
|
||
private String createSessionHeader(Session session) { | ||
SessionHeader sessionHeader = new SessionHeader(session.getUser().getName(), session.getSessionId()); | ||
ByteArrayOutputStream bos = new ByteArrayOutputStream(); | ||
ObjectOutputStream oos = null; | ||
try { | ||
oos = new ObjectOutputStream(bos); | ||
oos.writeObject(sessionHeader); | ||
oos.flush(); | ||
} catch (IOException e) { | ||
return ""; | ||
} | ||
return new String(org.apache.commons.codec.binary.Base64.encodeBase64(bos.toByteArray())); | ||
} | ||
|
||
private SessionHeader getSessionHeader(HttpServletRequest request) { | ||
String sessionAuth = request.getHeader("Session-Auth"); | ||
if (sessionAuth != null) { | ||
try { | ||
byte[] decoded = decodeBase64(sessionAuth); | ||
ObjectInputStream in = new ObjectInputStream(new ByteArrayInputStream(decoded)); | ||
return (SessionHeader) in.readObject(); | ||
} catch (Exception e) { | ||
return null; | ||
} | ||
} | ||
return null; | ||
} | ||
|
||
private boolean isAuthenticated(HttpServletRequest request) { | ||
SessionHeader sessionHeader = getSessionHeader(request); | ||
if (sessionHeader == null) return false; | ||
return sessionsRepository.findBySessionId(sessionHeader.getSessionId()) != null; | ||
} | ||
|
||
private User getUser(HttpServletRequest request) { | ||
SessionHeader sessionHeader = getSessionHeader(request); | ||
if (sessionHeader == null) return null; | ||
Session session = sessionsRepository.findBySessionId(sessionHeader.getSessionId()); | ||
if (session != null) return session.getUser(); | ||
return null; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package com.dashboardmanager.model; | ||
|
||
import jakarta.persistence.*; | ||
|
||
@Table(name = "Sessions") | ||
@Entity | ||
public class Session { | ||
|
||
@Id | ||
@Column | ||
@GeneratedValue(strategy = GenerationType.AUTO) | ||
private Integer id; | ||
|
||
@OneToOne | ||
private User user; | ||
|
||
@Column | ||
private String sessionId; | ||
|
||
public Integer getId() { return this.id; } | ||
public void setId(Integer id) { this.id = id; } | ||
|
||
public User getUser() { return this.user; } | ||
public void setUser(User user) { this.user = user; } | ||
|
||
public String getSessionId() { return this.sessionId; } | ||
public void setSessionId(String sessionId) { this.sessionId = sessionId; } | ||
|
||
} |
Oops, something went wrong.