sonar-findbugs icon indicating copy to clipboard operation
sonar-findbugs copied to clipboard

Rule findsecbugs:FILE_UPLOAD_FILENAME breaks JSON export (via API)

Open koseduhemak opened this issue 5 years ago • 1 comments

Issue Description

When exporting rules via API, the rule "findsecbugs:FILE_UPLOAD_FILENAME" (Security - Tainted filename read) generates invalid JSON shell.jsp\�expected.gif. The expected behavior is, that the string shell.jsp\�expected.gif would be fixed to be like shell.jsp\u0000expected.gif.

Problem seems to be the string "\u0000" which does not get properly encoded (and is in fact interpreted as unicode char). See "broken char" in the description above.

Just create a quality profile which contains the mentioned rule and try to export via API. Sample API call: mysonarqubeinstance.com/api/rules/search?activation=true&ps=500&p=1&qprofile=myQualityProfileWithTheRule&languages=java

Environment

Component Version
SonarQube Enterprise EditionVersion 8.4.2 (build 36762)
Sonar-FindBugs 4.0.0

Code (If needed)

{
            "key": "findsecbugs:FILE_UPLOAD_FILENAME",
            "repo": "findsecbugs",
            "name": "Security - Tainted filename read",
            "createdAt": "2017-09-07T15:58:10+0200",
            "htmlDesc":"<p>The filename provided by the FileUpload API can be tampered with by the client to reference unauthorized files.</p>\n<p>For example:</p>\n<ul>\n<li><code>\"../../../config/overide_file\"</code></li>\n<li><code>\"shell.jsp\�expected.gif\"</code></li>\n</ul>\n<p>Therefore, such values should not be passed directly to the filesystem API. If acceptable, the application should generate its\nown file names and use those. Otherwise, the provided filename should be properly validated to ensure it's properly structured,\ncontains no unauthorized path characters (e.g., / \\), and refers to an authorized file.</p>\n<br/>\n<p>\n<b>References</b><br/>\n<a href=\"https://blogs.securiteam.com/index.php/archives/1268\">Securiteam: File upload security recommendations</a><br/>\n<a href=\"https://cwe.mitre.org/data/definitions/22.html\">CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')</a><br/>\n<a href=\"http://projects.webappsec.org/w/page/13246952/Path%20Traversal\">WASC-33: Path Traversal</a><br/>\n<a href=\"https://www.owasp.org/index.php/Path_Traversal\">OWASP: Path Traversal</a><br/>\n<a href=\"https://capec.mitre.org/data/definitions/126.html\">CAPEC-126: Path Traversal</a><br/>\n<a href=\"https://cwe.mitre.org/data/definitions/22.html\">CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')</a>\n</p>",
            "mdDesc":"<p>The filename provided by the FileUpload API can be tampered with by the client to reference unauthorized files.</p>\n<p>For example:</p>\n<ul>\n<li><code>\"../../../config/overide_file\"</code></li>\n<li><code>\"shell.jsp\�expected.gif\"</code></li>\n</ul>\n<p>Therefore, such values should not be passed directly to the filesystem API. If acceptable, the application should generate its\nown file names and use those. Otherwise, the provided filename should be properly validated to ensure it's properly structured,\ncontains no unauthorized path characters (e.g., / \\), and refers to an authorized file.</p>\n<br/>\n<p>\n<b>References</b><br/>\n<a href=\"https://blogs.securiteam.com/index.php/archives/1268\">Securiteam: File upload security recommendations</a><br/>\n<a href=\"https://cwe.mitre.org/data/definitions/22.html\">CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')</a><br/>\n<a href=\"http://projects.webappsec.org/w/page/13246952/Path%20Traversal\">WASC-33: Path Traversal</a><br/>\n<a href=\"https://www.owasp.org/index.php/Path_Traversal\">OWASP: Path Traversal</a><br/>\n<a href=\"https://capec.mitre.org/data/definitions/126.html\">CAPEC-126: Path Traversal</a><br/>\n<a href=\"https://cwe.mitre.org/data/definitions/22.html\">CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')</a>\n</p>",
            "severity": "INFO",
            "status": "READY",
            "internalKey": "FILE_UPLOAD_FILENAME",
            "isTemplate": false,
            "tags": [],
            "sysTags": [
                "cwe",
                "owasp-a4",
                "wasc"
            ],
            "lang": "java",
            "langName": "Java",
            "params": [],
            "debtOverloaded": false,
            "remFnOverloaded": false,
            "scope": "MAIN",
            "isExternal": false,
            "type": "VULNERABILITY"
        },

koseduhemak avatar Oct 14 '20 14:10 koseduhemak

I tried reproducing the issue but seem to get "\u0000" as expected with this query: http://sonarinstance/api/rules/search?rule_key=findsecbugs:FILE_UPLOAD_FILENAME with sonarqube 8.4.2 and 8.9 so I think that issue was solved

gtoison avatar Jul 02 '22 14:07 gtoison

I'll close this issue as I'm not able to reproduce it, please do not hesitate to open a new one in case there's still a problem

gtoison avatar Aug 26 '22 15:08 gtoison