|
| 1 | +# GitHub Security Lab (GHSL) Vulnerability Report: `GHSL-2022-008` |
| 2 | + |
| 3 | +The [GitHub Security Lab](https://securitylab.github.com) team has identified a potential security vulnerability in [The OWASP Enterprise Security API](https://github.com/ESAPI/esapi-java-legacy). |
| 4 | + |
| 5 | +We are committed to working with you to help resolve this issue. In this report you will find everything you need to effectively coordinate a resolution of this issue with the GHSL team. |
| 6 | + |
| 7 | +If at any point you have concerns or questions about this process, please do not hesitate to reach out to us at `[email protected]` (please include `GHSL-2022-008` as a reference). |
| 8 | + |
| 9 | +If you are _NOT_ the correct point of contact for this report, please let us know! |
| 10 | + |
| 11 | +## Summary |
| 12 | + |
| 13 | +`getValidDirectoryPath` incorrectly treats sibling of a root directory as a child. |
| 14 | + |
| 15 | +## Product |
| 16 | + |
| 17 | +The OWASP Enterprise Security API |
| 18 | + |
| 19 | +## Tested Version |
| 20 | + |
| 21 | +v2.2.3.1 (The latest version of ["Legacy" 2.x branch](https://github.com/ESAPI/esapi-java-legacy#what-does-legacy-mean) as [ESAPI 3.x](https://github.com/ESAPI/esapi-java) is in early development and has no releases yet.) |
| 22 | + |
| 23 | +## Details |
| 24 | + |
| 25 | +### Issue: `getValidDirectoryPath` bypass (`GHSL-2022-008`) |
| 26 | + |
| 27 | +`parent` [1] - the third parameter in [`getValidDirectoryPath`](https://github.com/ESAPI/esapi-java-legacy/blob/07dd60a8cc9edf0c872d68ae8ae84c70f008d3d8/src/main/java/org/owasp/esapi/reference/DefaultValidator.java#L447-L483) is used to validate that the `input` [2] path is "inside specified parent" directory [3]. |
| 28 | + |
| 29 | +```java |
| 30 | +public String getValidDirectoryPath(String context, String input /* [2] */, File parent /* [1] */, boolean allowNull) throws ValidationException, IntrusionException { |
| 31 | + try { |
| 32 | + if (isEmpty(input)) { |
| 33 | + if (allowNull) return null; |
| 34 | + throw new ValidationException( context + ": Input directory path required", "Input directory path required: context=" + context + ", input=" + input, context ); |
| 35 | + } |
| 36 | + |
| 37 | + File dir = new File( input ); |
| 38 | + |
| 39 | + // check dir exists and parent exists and dir is inside parent |
| 40 | + if ( !dir.exists() ) { |
| 41 | + throw new ValidationException( context + ": Invalid directory name", "Invalid directory, does not exist: context=" + context + ", input=" + input ); |
| 42 | + } |
| 43 | + if ( !dir.isDirectory() ) { |
| 44 | + throw new ValidationException( context + ": Invalid directory name", "Invalid directory, not a directory: context=" + context + ", input=" + input ); |
| 45 | + } |
| 46 | + if ( !parent.exists() ) { |
| 47 | + throw new ValidationException( context + ": Invalid directory name", "Invalid directory, specified parent does not exist: context=" + context + ", input=" + input + ", parent=" + parent ); |
| 48 | + } |
| 49 | + if ( !parent.isDirectory() ) { |
| 50 | + throw new ValidationException( context + ": Invalid directory name", "Invalid directory, specified parent is not a directory: context=" + context + ", input=" + input + ", parent=" + parent ); |
| 51 | + } |
| 52 | + if ( !dir.getCanonicalPath().startsWith(parent.getCanonicalPath() ) ) { // <---------- [3] |
| 53 | + throw new ValidationException( context + ": Invalid directory name", "Invalid directory, not inside specified parent: context=" + context + ", input=" + input + ", parent=" + parent ); |
| 54 | + } |
| 55 | + |
| 56 | + // check canonical form matches input |
| 57 | + String canonicalPath = dir.getCanonicalPath(); |
| 58 | + String canonical = fileValidator.getValidInput( context, canonicalPath, "DirectoryName", 255, false); |
| 59 | + if ( !canonical.equals( input ) ) { |
| 60 | + throw new ValidationException( context + ": Invalid directory name", "Invalid directory name does not match the canonical path: context=" + context + ", input=" + input + ", canonical=" + canonical, context ); |
| 61 | + } |
| 62 | + return canonical; |
| 63 | + } catch (Exception e) { |
| 64 | + throw new ValidationException( context + ": Invalid directory name", "Failure to validate directory path: context=" + context + ", input=" + input, e, context ); |
| 65 | + } |
| 66 | +} |
| 67 | +``` |
| 68 | + |
| 69 | +If the result of `parent.getCanonicalPath()` is not slash terminated it allows for partial path traversal. |
| 70 | + |
| 71 | +Consider `"/usr/outnot".startsWith("/usr/out")`. The check is bypassed although `outnot` is not under the `out` directory. |
| 72 | +The terminating slash may be removed in various places. On Linux `println(new File("/var/"))` returns `/var`, but `println(new File("/var", "/"))` - `/var/`, however `println(new File("/var", "/").getCanonicalPath())` - `/var`. |
| 73 | + |
| 74 | +PoC (based on a unittest): |
| 75 | +```java |
| 76 | +Validator instance = ESAPI.validator(); |
| 77 | +ValidationErrorList errors = new ValidationErrorList(); |
| 78 | +assertTrue(instance.isValidDirectoryPath("poc", "/tmp/test2", new File("/tmp/test/"), false, errors)); |
| 79 | +assertEquals(0, errors.size()); |
| 80 | +``` |
| 81 | + |
| 82 | +#### Impact |
| 83 | + |
| 84 | +This issue allows to break out of expected directory. |
| 85 | + |
| 86 | +#### Remediation |
| 87 | + |
| 88 | +Consider using `getCanonicalFile().toPath().startsWith` to compare `Path`: |
| 89 | + |
| 90 | +```java |
| 91 | +if ( !dir.getCanonicalFile().toPath().startsWith(parent.getCanonicalFile().toPath() ) ) |
| 92 | +``` |
| 93 | + |
| 94 | +## GitHub Security Advisories |
| 95 | + |
| 96 | +We recommend you create a private [GitHub Security Advisory](https://help.github.com/en/github/managing-security-vulnerabilities/creating-a-security-advisory) for this finding. This also allows you to invite the GHSL team to collaborate and further discuss this finding in private before it is [published](https://help.github.com/en/github/managing-security-vulnerabilities/publishing-a-security-advisory). |
| 97 | + |
| 98 | +## Credit |
| 99 | + |
| 100 | +This issue was discovered and reported by GHSL team member [@JarLob (Jaroslav Lobačevski)](https://github.com/JarLob). |
| 101 | + |
| 102 | +## Contact |
| 103 | + |
| 104 | +You can contact the GHSL team at `[email protected]`, please include a reference to `GHSL-2022-008` in any communication regarding this issue. |
| 105 | + |
| 106 | +## Disclosure Policy |
| 107 | + |
| 108 | +This report is subject to our [coordinated disclosure policy](https://securitylab.github.com/advisories#policy). |
0 commit comments