Nemcio/GitBucket
1
0
Fork 0
mirror of https://github.com/gitbucket/gitbucket.git synced 2026-05-07 16:46:54 +02:00
Code Issues Packages Projects Releases Wiki Activity

Add new option to disable XSS protection in image rendering (#2806)

Browse Source
This commit is contained in:
Naoki Takezoe
2021-07-10 12:51:25 +09:00
committed by GitHub
parent e74db57b75
commit 5cbea281af
15 changed files with 78 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/main/resources/update/gitbucket-core_4.36.xml Normal file
View File

@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<changeSet>
<addColumn tableName="REPOSITORY">
<column name="SAFE_MODE" type="boolean" nullable="false" defaultValue="true"/>
</addColumn>
</changeSet>

1
src/main/scala/gitbucket/core/GitBucketCoreModule.scala
View File

@@ -118,4 +118,5 @@ object GitBucketCoreModule
new Version("4.35.1"),
new Version("4.35.2"),
new Version("4.35.3"),
new Version("4.36.0", new LiquibaseMigration("update/gitbucket-core_4.36.xml"))
)

2
src/main/scala/gitbucket/core/controller/ControllerBase.scala
View File

@@ -253,7 +253,7 @@ abstract class ControllerBase
repository: RepositoryService.RepositoryInfo
): Unit = {
JGitUtil.getObjectLoaderFromId(git, objectId) { loader =>
contentType = FileUtil.getSafeMimeType(path)
contentType = FileUtil.getSafeMimeType(path, repository.repository.options.safeMode)
if (loader.isLarge) {
response.setContentLength(loader.getSize.toInt)

6
src/main/scala/gitbucket/core/controller/FileUploadController.scala
View File

@@ -40,7 +40,7 @@ class FileUploadController
.writeByteArrayToFile(new File(getTemporaryDir(session.getId), FileUtil.checkFilename(fileId)), file.get())
session += Keys.Session.Upload(fileId) -> file.name
},
FileUtil.isImage
FileUtil.isImage(_)
)
}
@@ -191,9 +191,9 @@ class FileUploadController
}
}
private def execute(f: (FileItem, String) => Unit, mimeTypeChcker: (String) => Boolean) =
private def execute(f: (FileItem, String) => Unit, mimeTypeChecker: (String) => Boolean) =
fileParams.get("file") match {
case Some(file) if mimeTypeChcker(file.name) =>
case Some(file) if mimeTypeChecker(file.name) =>
val fileId = FileUtil.generateFileId
f(file, fileId)
contentType = "text/plain"

9
src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala
View File

@@ -57,7 +57,8 @@ trait RepositorySettingsControllerBase extends ControllerBase {
externalWikiUrl: Option[String],
allowFork: Boolean,
mergeOptions: Seq[String],
defaultMergeOption: String
defaultMergeOption: String,
safeMode: Boolean
)
val optionsForm = mapping(
@@ -69,7 +70,8 @@ trait RepositorySettingsControllerBase extends ControllerBase {
"externalWikiUrl" -> trim(label("External Wiki URL", optional(text(maxlength(200))))),
"allowFork" -> trim(label("Allow Forking", boolean())),
"mergeOptions" -> mergeOptions,
"defaultMergeOption" -> trim(label("Default merge strategy", text(required)))
"defaultMergeOption" -> trim(label("Default merge strategy", text(required))),
"safeMode" -> trim(label("XSS protection", boolean()))
)(OptionsForm.apply).verifying { form =>
if (!form.mergeOptions.contains(form.defaultMergeOption)) {
Seq("defaultMergeOption" -> s"This merge strategy isn't enabled.")
@@ -150,7 +152,8 @@ trait RepositorySettingsControllerBase extends ControllerBase {
form.externalWikiUrl,
form.allowFork,
form.mergeOptions,
form.defaultMergeOption
form.defaultMergeOption,
form.safeMode
)
flash.update("info", "Repository settings has been updated.")
redirect(s"/${repository.owner}/${repository.name}/settings/options")

6
src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala
View File

@@ -423,7 +423,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
repository = repository,
pathList = paths.take(paths.size - 1).toList,
fileName = Some(paths.last),
content = JGitUtil.getContentInfo(git, path, objectId),
content = JGitUtil.getContentInfo(git, path, objectId, repository.repository.options.safeMode),
protectedBranch = protectedBranch,
commit = revCommit.getName,
newLineMode = info.newLineMode,
@@ -448,7 +448,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
repository = repository,
pathList = paths.take(paths.size - 1).toList,
fileName = paths.last,
content = JGitUtil.getContentInfo(git, path, objectId),
content = JGitUtil.getContentInfo(git, path, objectId, repository.repository.options.safeMode),
commit = revCommit.getName
)
} getOrElse NotFound()
@@ -693,7 +693,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
branch = id,
repository = repository,
pathList = path.split("/").toList,
content = JGitUtil.getContentInfo(git, path, objectId),
content = JGitUtil.getContentInfo(git, path, objectId, repository.repository.options.safeMode),
latestCommit = new JGitUtil.CommitInfo(JGitUtil.getLastModifiedCommit(git, revCommit, path)),
hasWritePermission = hasDeveloperRole(repository.owner, repository.name, context.loginAccount),
isBlame = request.paths(2) == "blame",

15
src/main/scala/gitbucket/core/model/Repository.scala
View File

@@ -24,6 +24,7 @@ trait RepositoryComponent extends TemplateComponent { self: Profile =>
val allowFork = column[Boolean]("ALLOW_FORK")
val mergeOptions = column[String]("MERGE_OPTIONS")
val defaultMergeOption = column[String]("DEFAULT_MERGE_OPTION")
val safeMode = column[Boolean]("SAFE_MODE")
def * =
(
@@ -41,7 +42,16 @@ trait RepositoryComponent extends TemplateComponent { self: Profile =>
parentUserName.?,
parentRepositoryName.?
),
(issuesOption, externalIssuesUrl.?, wikiOption, externalWikiUrl.?, allowFork, mergeOptions, defaultMergeOption)
(
issuesOption,
externalIssuesUrl.?,
wikiOption,
externalWikiUrl.?,
allowFork,
mergeOptions,
defaultMergeOption,
safeMode
)
).shaped.<>(
{
case (repository, options) =>
@@ -112,5 +122,6 @@ case class RepositoryOptions(
externalWikiUrl: Option[String],
allowFork: Boolean,
mergeOptions: String,
defaultMergeOption: String
defaultMergeOption: String,
safeMode: Boolean
)

10
src/main/scala/gitbucket/core/service/RepositoryService.scala
View File

@@ -60,7 +60,8 @@ trait RepositoryService {
externalWikiUrl = None,
allowFork = true,
mergeOptions = "merge-commit,squash,rebase",
defaultMergeOption = "merge-commit"
defaultMergeOption = "merge-commit",
safeMode = true
)
)
@@ -460,7 +461,7 @@ trait RepositoryService {
.filter { case (t1, t2) => t2.removed === false.bind }
.map { case (t1, t2) => t1 }
// for Normal Users
case Some(x) if (!x.isAdmin || limit) =>
case Some(x) =>
Repositories
.join(Accounts)
.on(_.userName === _.userName)
@@ -550,7 +551,8 @@ trait RepositoryService {
externalWikiUrl: Option[String],
allowFork: Boolean,
mergeOptions: Seq[String],
defaultMergeOption: String
defaultMergeOption: String,
safeMode: Boolean
)(implicit s: Session): Unit = {
Repositories
@@ -566,6 +568,7 @@ trait RepositoryService {
r.allowFork,
r.mergeOptions,
r.defaultMergeOption,
r.safeMode,
r.updatedDate
)
}
@@ -579,6 +582,7 @@ trait RepositoryService {
allowFork,
mergeOptions.mkString(","),
defaultMergeOption,
safeMode,
currentDate
)
}

13
src/main/scala/gitbucket/core/util/FileUtil.scala
View File

@@ -24,13 +24,18 @@ object FileUtil {
}
}
def getSafeMimeType(name: String): String = {
getMimeType(name)
def getSafeMimeType(name: String, safeMode: Boolean = true): String = {
val mimeType = getMimeType(name)
.replace("text/html", "text/plain")
.replace("image/svg+xml", "text/plain; charset=UTF-8")
if (safeMode) {
mimeType.replace("image/svg+xml", "text/plain; charset=UTF-8")
} else {
mimeType
}
}
def isImage(name: String): Boolean = getSafeMimeType(name).startsWith("image/")
def isImage(name: String, safeMode: Boolean = true): Boolean = getSafeMimeType(name, safeMode).startsWith("image/")
def isLarge(size: Long): Boolean = (size > 1024 * 1000)

4
src/main/scala/gitbucket/core/util/JGitUtil.scala
View File

@@ -1047,13 +1047,13 @@ object JGitUtil {
!loader.isLarge && new String(loader.getBytes(), "UTF-8").startsWith("version https://git-lfs.github.com/spec/v1")
}
def getContentInfo(git: Git, path: String, objectId: ObjectId): ContentInfo = {
def getContentInfo(git: Git, path: String, objectId: ObjectId, safeMode: Boolean): ContentInfo = {
// Viewer
Using.resource(git.getRepository.getObjectDatabase) { db =>
val loader = db.open(objectId)
val isLfs = isLfsPointer(loader)
val large = FileUtil.isLarge(loader.getSize)
val viewer = if (FileUtil.isImage(path)) "image" else if (large) "large" else "other"
val viewer = if (FileUtil.isImage(path, safeMode)) "image" else if (large) "large" else "other"
val bytes = if (viewer == "other") JGitUtil.getContentFromId(git, objectId, false) else None
val size = Some(getContentSize(loader))

9
src/main/twirl/gitbucket/core/settings/options.scala.html
View File

@@ -33,8 +33,6 @@
You choose who can see and commit to this repository.
</div>
</label>
</fieldset>
<fieldset class="form-group">
<label class="checkbox" for="allowFork">
<input type="checkbox" id="allowFork" name="allowFork"@if(repository.repository.options.allowFork){ checked}/>
Forks<br>
@@ -42,6 +40,13 @@
Allow users who can access this repository to fork it.
</div>
</label>
<label class="checkbox" for="safeMode">
<input type="checkbox" id="safeMode" name="safeMode"@if(repository.repository.options.safeMode){ checked}/>
XSS protection<br>
<div class="normal muted">
Render any images in the repository viewer. Don't disable this option in the public environment.
</div>
</label>
</fieldset>
</div>
</div>

4
src/test/scala/gitbucket/core/api/ApiSpecModels.scala
View File

@@ -2,7 +2,6 @@ package gitbucket.core.api
import java.util.{Calendar, Date, TimeZone}
import gitbucket.core.api.ApiBranchProtection.EnforcementLevel
import gitbucket.core.model._
import gitbucket.core.plugin.PluginInfo
import gitbucket.core.service.ProtectedBranchService.ProtectedBranchInfo
@@ -69,7 +68,8 @@ object ApiSpecModels {
externalWikiUrl = Some("https://external.com/gitbucket"),
allowFork = true,
mergeOptions = "merge-commit,squash,rebase",
defaultMergeOption = "merge-commit"
defaultMergeOption = "merge-commit",
safeMode = true
)
)

3
src/test/scala/gitbucket/core/service/MergeServiceSpec.scala
View File

@@ -239,7 +239,8 @@ class MergeServiceSpec extends AnyFunSpec with ServiceSpecBase {
externalWikiUrl = None,
allowFork = true,
mergeOptions = "merge-commit,squash,rebase",
defaultMergeOption = "merge-commit"
defaultMergeOption = "merge-commit",
safeMode = true
)
),
issueCount = 0,

15
src/test/scala/gitbucket/core/util/FileUtilSpec.scala Normal file
View File

@@ -0,0 +1,15 @@
package gitbucket.core.util
import org.scalatest.funsuite.AnyFunSuite
class FileUtilSpec extends AnyFunSuite {
test("getSafeMimeType") {
val contentType1 = FileUtil.getSafeMimeType("test.svg", true)
assert(contentType1 == "text/plain; charset=UTF-8")
val contentType2 = FileUtil.getSafeMimeType("test.svg", false)
assert(contentType2 == "image/svg+xml")
}
}

2
src/test/scala/gitbucket/core/util/GitSpecUtil.scala
View File

@@ -93,7 +93,7 @@ object GitSpecUtil {
}
_getPathObjectId
}
JGitUtil.getContentInfo(git, path, objectId)
JGitUtil.getContentInfo(git, path, objectId, true)
}
def mergeAndCommit(git: Git, into: String, branch: String, message: String = null): Unit = {