Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TEST] frogbot test #190

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

[TEST] frogbot test #190

wants to merge 10 commits into from

Conversation

lucboudreau
Copy link
Member

No description provided.

@lucboudreau lucboudreau requested a review from a team as a code owner August 30, 2024 15:12
@buildguy

This comment has been minimized.

@buildguy

This comment has been minimized.

@buildguy

This comment has been minimized.

@buildguy

This comment has been minimized.

@buildguy

This comment has been minimized.

@buildguy

This comment has been minimized.

@buildguy

This comment has been minimized.

@buildguy
Copy link
Collaborator

👍 Frogbot scanned this pull request and found that it did not add vulnerable dependencies.


@buildguy

This comment has been minimized.

@buildguy

This comment has been minimized.

@buildguy

This comment has been minimized.

@pentaho pentaho deleted a comment from buildguy Oct 2, 2024
@pentaho pentaho deleted a comment from buildguy Oct 2, 2024
@pentaho pentaho deleted a comment from buildguy Oct 2, 2024
@buildguy

This comment has been minimized.

@buildguy

This comment has been minimized.

@buildguy
Copy link
Collaborator

buildguy commented Oct 2, 2024

❌ Build failed in 44s

Build command:

mvn clean verify -B -e -Daudit -Djs.no.sandbox

❗ No tests found!

Errors:

Filtered log (click to expand)

script returned exit code 1

ℹ️ This is an automatic message

@buildguy
Copy link
Collaborator

buildguy commented Oct 2, 2024

🚨 Frogbot scanned this pull request and found the below:


@@ -163,6 +166,7 @@ private String getFileContents(File file) {
protected File getLastFile() {
File file = new File(".schemaInfo");
if (file.exists()) {
// Some comments here and there
String path = getFileContents(file).replaceAll("\n", "");
return new File(path);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Static Application Security Testing (SAST) Vulnerability

Severity Finding

High
Path Traversal
Full description

Overview

Path traversal, also known as directory traversal, is a type of
vulnerability that allows an attacker to access files or directories on a
computer or device that are outside of the intended directory.
Allowing arbitrary read access can allow the attacker to read sensitive
files, such as configuration files or sensitive data, potentially leading
data loss or even system compromise. Allowing arbitrary write access is
more severe and in most cases leads to arbitrary code execution, via
editing important system files or sensitive data.

Vulnerable example

public class path_traversaLvuln {
    protected void doGet(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        String DOCS_FOLDER = "/srv/www/docs";
        String docName = request.getParameter("doc");
        Path docPath = Paths.get(DOCS_FOLDER, docName);
        File docFile = docPath.toFile();
        FileUtils.copyFile(docFile, response.getOutputStream());
    }
}

In this example, an attacker can inject a back-path, that will get anywhere
in the system, using "../../".

Remediation

public class path_traversal_safe {
    protected void doGet(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        String DOCS_FOLDER = "/srv/www/docs";
        String docName = request.getParameter("doc");
        Path docPath = Paths.get(DOCS_FOLDER, docName);
+         Path normDocPath = docPath.normalize();
+         // Make sure the canonical path resides in the desired dir
+         if (normDocPath.startsWith(DOCS_FOLDER)) {
            File docFile = docPath.toFile();
            FileUtils.copyFile(docFile, response.getOutputStream());
+         }
    }
}

By checking that the folder name still starts with the predefined prefix, we
make sure that the attacker is not able to back-path outside of the allowed
folder.

Code Flows
Vulnerable data flow analysis result

↘️ br.readLine() (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 153)

↘️ line (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 153)

↘️ line (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 154)

↘️ line + "\n" (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 154)

↘️ sb.append(line + "\n") (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 154)

↘️ sb (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 154)

↘️ sb (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 156)

↘️ sb.toString() (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 156)

↘️ return sb.toString(); (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 156)

↘️ getFileContents(file) (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 170)

↘️ getFileContents(file).replaceAll("\n", "") (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 170)

↘️ path (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 170)

↘️ path (at pentaho-aggdesigner-ui/src/main/java/org/pentaho/aggdes/ui/ext/impl/MondrianFileSchemaProvider.java line 171)


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants