Skip to content

Commit b7c8767

Browse files
antoniaclaude
andcommitted
fix: patch SSRF and Path Traversal vulnerabilities in subtitle download
Security fixes for CVE-worthy vulnerabilities (CVSS 4.0: 8.7): - ZipHelper: Reject path traversal characters (.., /, \) in filenames - ZipHelper: Add canonical path validation to prevent directory escape - OpensubtitlesService: Whitelist URLs to opensubtitles.org domains only - SubtitlesController: Add input validation as defense in depth This prevents authenticated attackers from: - Writing arbitrary files via path traversal in subFileName parameter - Accessing internal services via SSRF in subDownloadLink parameter Reported by: Valentin Lobstein CWE-22 (Path Traversal) + CWE-918 (SSRF) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 7de904b commit b7c8767

File tree

5 files changed

+281
-11
lines changed

5 files changed

+281
-11
lines changed

grails-app/controllers/streama/SubtitlesController.groovy

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,33 @@ class SubtitlesController {
4646
def subtitleLink = params.subDownloadLink
4747
def subtitleLang = params.subLang
4848
def videoId = params.videoId
49-
if (opensubtitlesService.downloadSubtitles(subtitleName, subtitleLink, subtitleLang, videoId)) {
50-
respond status: OK
51-
} else {
52-
respond status: BAD_REQUEST
49+
50+
// Security fix: Input validation at controller level (defense in depth)
51+
// Validate required parameters are present
52+
if (!subtitleName || !subtitleLink || !subtitleLang || !videoId) {
53+
response.status = BAD_REQUEST.value()
54+
respond([error: true, message: "Missing required parameters"])
55+
return
56+
}
57+
58+
// Validate subtitleName doesn't contain path traversal characters
59+
if (subtitleName.contains("..") || subtitleName.contains("/") || subtitleName.contains("\\")) {
60+
log.warn("Security: Blocked potential path traversal in subtitle filename: ${subtitleName}")
61+
response.status = BAD_REQUEST.value()
62+
respond([error: true, message: "Invalid subtitle filename"])
63+
return
64+
}
65+
66+
try {
67+
if (opensubtitlesService.downloadSubtitles(subtitleName, subtitleLink, subtitleLang, videoId)) {
68+
respond status: OK
69+
} else {
70+
respond status: BAD_REQUEST
71+
}
72+
} catch (SecurityException e) {
73+
log.error("Security violation in subtitle download: ${e.message}")
74+
response.status = BAD_REQUEST.value()
75+
respond([error: true, message: e.message])
5376
}
5477
}
5578

grails-app/services/streama/OpensubtitlesService.groovy

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import org.springframework.http.HttpMethod
77
import org.springframework.http.ResponseEntity
88

99
import javax.activation.MimetypesFileTypeMap
10+
import java.net.MalformedURLException
1011

1112
@Transactional
1213
class OpensubtitlesService {
@@ -78,9 +79,52 @@ class OpensubtitlesService {
7879
return response
7980
}
8081

82+
// Allowed domains for subtitle downloads (SSRF mitigation - CWE-918)
83+
private static final List<String> ALLOWED_SUBTITLE_HOSTS = [
84+
"dl.opensubtitles.org",
85+
"www.opensubtitles.org",
86+
"opensubtitles.org",
87+
"vip.opensubtitles.org"
88+
]
89+
90+
/**
91+
* Validates that a URL is from an allowed subtitle provider.
92+
* This prevents SSRF attacks by ensuring only whitelisted domains are accessed.
93+
*/
94+
private boolean isUrlAllowed(String urlString) {
95+
try {
96+
URL parsedUrl = new URL(urlString)
97+
String host = parsedUrl.getHost()?.toLowerCase()
98+
String protocol = parsedUrl.getProtocol()?.toLowerCase()
99+
100+
// Only allow HTTPS (or HTTP for backwards compatibility with opensubtitles)
101+
if (protocol != "https" && protocol != "http") {
102+
log.warn("Blocked subtitle download attempt with disallowed protocol: ${protocol}")
103+
return false
104+
}
105+
106+
// Check if host is in allowlist
107+
if (!ALLOWED_SUBTITLE_HOSTS.contains(host)) {
108+
log.warn("Blocked subtitle download attempt from disallowed host: ${host}")
109+
return false
110+
}
111+
112+
return true
113+
} catch (MalformedURLException e) {
114+
log.warn("Blocked subtitle download attempt with malformed URL: ${urlString}")
115+
return false
116+
}
117+
}
118+
81119
def downloadSubtitles(String subtitleName, String url, String subtitleLanguage, videoId) {
82120
def videoInstance = Video.findById(videoId)
83121
if (videoInstance != null) {
122+
// Security fix: Validate URL before fetching (SSRF mitigation - CWE-918)
123+
if (!isUrlAllowed(url)) {
124+
log.error("Security: Blocked subtitle download from unauthorized URL: ${url}")
125+
throw new SecurityException("Subtitle downloads are only allowed from opensubtitles.org")
126+
}
127+
84128
def stagingDir = uploadService.getDir().uploadDir.toString()
85129
def filename = url.tokenize('/')[-1]
86130
def filePath = new java.io.File("$stagingDir/$filename")

src/main/groovy/streama/ZipHelper.groovy

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,30 @@ class ZipHelper {
1414
throw new FileNotFoundException("Failed to unzip file. File not found.")
1515
}
1616

17+
// Security fix: Reject path traversal attempts (CWE-22)
18+
// Check for path traversal characters BEFORE sanitization to detect attacks
19+
if (originalFileName.contains("..") || originalFileName.contains("/") || originalFileName.contains("\\")) {
20+
throw new SecurityException("Path traversal attempt detected in filename: ${originalFileName}")
21+
}
22+
23+
// Sanitize filename as defense-in-depth
24+
String sanitizedFileName = new File(originalFileName).getName()
25+
if (sanitizedFileName.isEmpty() || sanitizedFileName.startsWith(".")) {
26+
throw new SecurityException("Invalid filename: filename cannot be empty or start with a dot")
27+
}
28+
29+
// Build target path and validate it's within staging directory
30+
File stagingDirFile = new File(stagingDir)
31+
File targetFile = new File(stagingDirFile, sanitizedFileName)
32+
String canonicalStagingDir = stagingDirFile.getCanonicalPath()
33+
String canonicalTargetPath = targetFile.getCanonicalPath()
34+
35+
if (!canonicalTargetPath.startsWith(canonicalStagingDir + File.separator)) {
36+
throw new SecurityException("Path traversal attempt detected: file path escapes staging directory")
37+
}
38+
1739
GZIPInputStream gzis = new GZIPInputStream(new FileInputStream(filePath))
18-
FileOutputStream fos = new FileOutputStream("$stagingDir/$originalFileName")
40+
FileOutputStream fos = new FileOutputStream(targetFile)
1941

2042
int len
2143
while ((len = gzis.read(buffer)) > 0) {
@@ -24,12 +46,20 @@ class ZipHelper {
2446

2547
gzis.close()
2648
fos.close()
27-
def file = new java.io.File("$stagingDir/$originalFileName")
28-
def sha256Hex = file.newInputStream().withStream { stream -> DigestUtils.sha256Hex(stream) }
29-
def index = originalFileName.lastIndexOf('.')
30-
String extension = originalFileName[index..-1]
31-
def newFile = new java.io.File("$stagingDir/$sha256Hex$extension")
32-
file.renameTo(newFile)
49+
50+
// Use the validated targetFile instead of reconstructing path
51+
def sha256Hex = targetFile.newInputStream().withStream { stream -> DigestUtils.sha256Hex(stream) }
52+
def index = sanitizedFileName.lastIndexOf('.')
53+
String extension = sanitizedFileName[index..-1]
54+
55+
// Validate the new filename is also safe
56+
File newFile = new File(stagingDirFile, "$sha256Hex$extension")
57+
String canonicalNewPath = newFile.getCanonicalPath()
58+
if (!canonicalNewPath.startsWith(canonicalStagingDir + File.separator)) {
59+
throw new SecurityException("Path traversal attempt detected in renamed file")
60+
}
61+
62+
targetFile.renameTo(newFile)
3363
return "$sha256Hex$extension"
3464
}
3565
}
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package streama
2+
3+
import grails.test.mixin.TestFor
4+
import spock.lang.Specification
5+
import spock.lang.Unroll
6+
7+
@TestFor(OpensubtitlesService)
8+
class OpensubtitlesServiceSecuritySpec extends Specification {
9+
10+
def setup() {
11+
// Mock dependencies
12+
service.uploadService = Mock(UploadService) {
13+
getDir() >> [uploadDir: File.createTempDir()]
14+
}
15+
}
16+
17+
@Unroll
18+
def "allows valid opensubtitles URL: #url"() {
19+
expect:
20+
service.isUrlAllowed(url) == true
21+
22+
where:
23+
url << [
24+
"http://dl.opensubtitles.org/en/download/sub/12345",
25+
"https://dl.opensubtitles.org/en/download/sub/12345",
26+
"http://www.opensubtitles.org/download/sub/67890",
27+
"https://opensubtitles.org/download/sub/11111",
28+
"http://vip.opensubtitles.org/download/sub/22222"
29+
]
30+
}
31+
32+
@Unroll
33+
def "blocks SSRF attack URL: #url"() {
34+
expect:
35+
service.isUrlAllowed(url) == false
36+
37+
where:
38+
url << [
39+
// Internal network SSRF
40+
"http://localhost/admin",
41+
"http://127.0.0.1/admin",
42+
"http://192.168.1.1/admin",
43+
"http://10.0.0.1/internal",
44+
"http://169.254.169.254/latest/meta-data/", // AWS metadata
45+
46+
// External malicious hosts
47+
"http://evil.com/payload.gz",
48+
"http://attacker.org/shell.gz",
49+
50+
// Protocol attacks
51+
"file:///etc/passwd",
52+
"ftp://attacker.com/payload",
53+
"gopher://localhost:25/",
54+
55+
// Subdomain bypass attempts
56+
"http://opensubtitles.org.evil.com/",
57+
"http://dl.opensubtitles.org.attacker.com/",
58+
59+
// Malformed URLs
60+
"",
61+
"not-a-url",
62+
"javascript:alert(1)"
63+
]
64+
}
65+
66+
def "downloadSubtitles throws SecurityException for blocked URL"() {
67+
given:
68+
def video = Mock(Video) {
69+
getId() >> 1L
70+
}
71+
Video.metaClass.static.findById = { id -> video }
72+
73+
when:
74+
service.downloadSubtitles("subtitle.srt", "http://evil.com/payload.gz", "en", 1L)
75+
76+
then:
77+
thrown(SecurityException)
78+
}
79+
}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package streama
2+
3+
import spock.lang.Specification
4+
import spock.lang.Unroll
5+
6+
class ZipHelperSecuritySpec extends Specification {
7+
8+
def stagingDir
9+
def testFile
10+
11+
def setup() {
12+
// Create a temporary staging directory for tests
13+
stagingDir = File.createTempDir("streama-test-", "-staging")
14+
15+
// Create a test gzip file
16+
testFile = new File(stagingDir, "test.gz")
17+
def content = "test content".bytes
18+
new java.util.zip.GZIPOutputStream(new FileOutputStream(testFile)).withStream { gz ->
19+
gz.write(content)
20+
}
21+
}
22+
23+
def cleanup() {
24+
// Clean up temp files
25+
stagingDir?.deleteDir()
26+
}
27+
28+
def "happy path - valid filename extracts correctly"() {
29+
when:
30+
def result = ZipHelper.unzipFile(stagingDir.absolutePath, "test.gz", "subtitle.srt")
31+
32+
then:
33+
result.endsWith(".srt")
34+
// File should exist in staging dir
35+
new File(stagingDir, result).exists()
36+
}
37+
38+
def "happy path - filename with spaces works"() {
39+
when:
40+
def result = ZipHelper.unzipFile(stagingDir.absolutePath, "test.gz", "my subtitle file.srt")
41+
42+
then:
43+
result.endsWith(".srt")
44+
new File(stagingDir, result).exists()
45+
}
46+
47+
@Unroll
48+
def "blocks path traversal attack with filename: #maliciousName"() {
49+
when:
50+
ZipHelper.unzipFile(stagingDir.absolutePath, "test.gz", maliciousName)
51+
52+
then:
53+
def e = thrown(SecurityException)
54+
e.message.contains("Path traversal") || e.message.contains("Invalid filename")
55+
56+
where:
57+
maliciousName << [
58+
"../../../../etc/passwd",
59+
"../../../tmp/pwned.txt",
60+
"..\\..\\..\\windows\\system32\\config\\sam",
61+
"/etc/passwd",
62+
"\\windows\\system32\\config\\sam",
63+
"../subtitle.srt",
64+
"foo/../bar.srt",
65+
"foo/bar.srt"
66+
]
67+
}
68+
69+
def "blocks empty filename"() {
70+
when:
71+
ZipHelper.unzipFile(stagingDir.absolutePath, "test.gz", "")
72+
73+
then:
74+
def e = thrown(SecurityException)
75+
e.message.contains("Invalid filename")
76+
}
77+
78+
def "blocks dotfile filename"() {
79+
when:
80+
ZipHelper.unzipFile(stagingDir.absolutePath, "test.gz", ".htaccess")
81+
82+
then:
83+
def e = thrown(SecurityException)
84+
e.message.contains("Invalid filename")
85+
}
86+
87+
def "blocks double-dot filename"() {
88+
when:
89+
ZipHelper.unzipFile(stagingDir.absolutePath, "test.gz", "..hidden")
90+
91+
then:
92+
def e = thrown(SecurityException)
93+
}
94+
}

0 commit comments

Comments
 (0)