mirror of
https://github.com/bitfireAT/davx5-ose
synced 2024-07-23 19:50:18 +00:00
Security: check/fix DebugInfoActivity: Uncontrolled data used in path expression (bitfireAT/davx5#267)
* Passing file name to `EXTRA_LOG_FILE` instead of path. Replaces illegal chars Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me> * Removed log file extra Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me> * Removed log file extra Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me> * Added more error logging Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me> * Update KDoc, remove strings * Moved ViewModel initialization to Factory Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me> * Changed ViewModel factory Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me> * Updated annotations Signed-off-by: Arnau Mora <arnyminerz@proton.me> * Handle EXTRA_LOGS correctly --------- Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me> Signed-off-by: Arnau Mora <arnyminerz@proton.me> Co-authored-by: Ricki Hirner <hirner@bitfire.at>
This commit is contained in:
Arnau Mora
Ricki Hirner
parent
3ba463dbd4
commit
11c8866614
|
|
@ -22,7 +22,7 @@ import at.bitfire.davdroid.ui.NotificationUtils
|
|||
import at.bitfire.davdroid.ui.NotificationUtils.notifyIfPossible
|
||||
import java.io.File
|
||||
import java.io.IOException
|
||||
import java.util.*
|
||||
import java.util.Date
|
||||
import java.util.logging.FileHandler
|
||||
import java.util.logging.Level
|
||||
|
||||
|
|
@ -75,8 +75,7 @@ object Logger : SharedPreferences.OnSharedPreferenceChangeListener {
|
|||
val nm = NotificationManagerCompat.from(context)
|
||||
// log to external file according to preferences
|
||||
if (logToFile) {
|
||||
val logDir = debugDir() ?: return
|
||||
val logFile = File(logDir, "davx5-log.txt")
|
||||
val logFile = getDebugLogFile() ?: return log.warning("Log file could not be retrieved.")
|
||||
if (logFile.createNewFile())
|
||||
logFile.writeText("Log file created at ${Date()}; PID ${Process.myPid()}; UID ${Process.myUid()}\n")
|
||||
|
||||
|
|
@ -96,7 +95,6 @@ object Logger : SharedPreferences.OnSharedPreferenceChangeListener {
|
|||
.setOngoing(true)
|
||||
|
||||
val shareIntent = DebugInfoActivity.IntentBuilder(context)
|
||||
.withLogFile(logFile)
|
||||
.newTask()
|
||||
.share()
|
||||
val pendingShare = PendingIntent.getActivity(context, 0, shareIntent, PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE)
|
||||
|
|
@ -129,7 +127,13 @@ object Logger : SharedPreferences.OnSharedPreferenceChangeListener {
|
|||
}
|
||||
|
||||
|
||||
private fun debugDir(): File? {
|
||||
/**
|
||||
* Creates (when necessary) and returns the directory where all the debug files (such as log files) are stored.
|
||||
* Must match the contents of `res/xml/debug.paths.xml`.
|
||||
*
|
||||
* @return The directory where all debug info are stored, or `null` if the directory couldn't be created successfully.
|
||||
*/
|
||||
fun debugDir(): File? {
|
||||
val dir = File(context.filesDir, "debug")
|
||||
if (dir.exists() && dir.isDirectory)
|
||||
return dir
|
||||
|
|
@ -141,4 +145,14 @@ object Logger : SharedPreferences.OnSharedPreferenceChangeListener {
|
|||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* The file (in [debugDir]) where verbose logs are stored.
|
||||
*
|
||||
* @return The file where verbose logs are stored, or `null` if there's no [debugDir].
|
||||
*/
|
||||
fun getDebugLogFile(): File? {
|
||||
val logDir = debugDir() ?: return null
|
||||
return File(logDir, "davx5-log.txt")
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -27,7 +27,6 @@ import android.provider.CalendarContract
|
|||
import android.provider.ContactsContract
|
||||
import android.view.View
|
||||
import androidx.activity.viewModels
|
||||
import androidx.annotation.UiThread
|
||||
import androidx.appcompat.app.AppCompatActivity
|
||||
import androidx.core.app.NotificationManagerCompat
|
||||
import androidx.core.app.ShareCompat
|
||||
|
|
@ -38,6 +37,8 @@ import androidx.core.content.pm.PackageInfoCompat
|
|||
import androidx.databinding.DataBindingUtil
|
||||
import androidx.lifecycle.AndroidViewModel
|
||||
import androidx.lifecycle.MutableLiveData
|
||||
import androidx.lifecycle.ViewModel
|
||||
import androidx.lifecycle.ViewModelProvider
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import androidx.work.WorkManager
|
||||
import androidx.work.WorkQuery
|
||||
|
|
@ -60,8 +61,10 @@ import at.bitfire.ical4android.TaskProvider.ProviderName
|
|||
import at.bitfire.ical4android.util.MiscUtils.ContentProviderClientHelper.closeCompat
|
||||
import at.techbee.jtx.JtxContract
|
||||
import com.google.android.material.snackbar.Snackbar
|
||||
import dagger.assisted.Assisted
|
||||
import dagger.assisted.AssistedFactory
|
||||
import dagger.assisted.AssistedInject
|
||||
import dagger.hilt.android.AndroidEntryPoint
|
||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.launch
|
||||
import okhttp3.HttpUrl
|
||||
|
|
@ -86,6 +89,16 @@ import at.bitfire.ical4android.util.MiscUtils.UriHelper.asSyncAdapter as asCalen
|
|||
import at.bitfire.vcard4android.Utils.asSyncAdapter as asContactsSyncAdapter
|
||||
import at.techbee.jtx.JtxContract.asSyncAdapter as asJtxSyncAdapter
|
||||
|
||||
/**
|
||||
* Debug info activity. Provides verbose information for debugging and support. Should enable users
|
||||
* to debug problems themselves, but also to send it to the support.
|
||||
*
|
||||
* Important use cases to test:
|
||||
*
|
||||
* - debug info from App settings / Debug info (should provide debug info)
|
||||
* - login with some broken login URL (should provide debug info + logs; check logs, too)
|
||||
* - enable App settings / Verbose logs, then open debug info activity (should provide debug info + logs; check logs, too)
|
||||
*/
|
||||
@AndroidEntryPoint
|
||||
class DebugInfoActivity : AppCompatActivity() {
|
||||
|
||||
|
|
@ -102,9 +115,6 @@ class DebugInfoActivity : AppCompatActivity() {
|
|||
/** dump of local resource related to the problem (plain-text [String]) */
|
||||
private const val EXTRA_LOCAL_RESOURCE = "localResource"
|
||||
|
||||
/** logs related to the problem (path to log file, plain-text [String]) */
|
||||
private const val EXTRA_LOG_FILE = "logFile"
|
||||
|
||||
/** logs related to the problem (plain-text [String]) */
|
||||
private const val EXTRA_LOGS = "logs"
|
||||
|
||||
|
|
@ -115,14 +125,19 @@ class DebugInfoActivity : AppCompatActivity() {
|
|||
const val FILE_LOGS = "logs.txt"
|
||||
}
|
||||
|
||||
private val model by viewModels<ReportModel>()
|
||||
@Inject lateinit var modelFactory: ReportModel.Factory
|
||||
private val model by viewModels<ReportModel> {
|
||||
object: ViewModelProvider.Factory {
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
override fun <T : ViewModel> create(modelClass: Class<T>): T =
|
||||
modelFactory.create(intent.extras) as T
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
override fun onCreate(savedInstanceState: Bundle?) {
|
||||
super.onCreate(savedInstanceState)
|
||||
|
||||
model.generate(intent.extras)
|
||||
|
||||
val binding = DataBindingUtil.setContentView<ActivityDebugInfoBinding>(this, R.layout.activity_debug_info)
|
||||
binding.model = model
|
||||
binding.lifecycleOwner = this
|
||||
|
|
@ -221,17 +236,21 @@ class DebugInfoActivity : AppCompatActivity() {
|
|||
}
|
||||
|
||||
|
||||
@HiltViewModel
|
||||
class ReportModel @Inject constructor(
|
||||
val context: Application
|
||||
class ReportModel @AssistedInject constructor (
|
||||
val context: Application,
|
||||
@Assisted extras: Bundle?
|
||||
) : AndroidViewModel(context) {
|
||||
|
||||
@AssistedFactory
|
||||
interface Factory {
|
||||
fun create(extras: Bundle?): ReportModel
|
||||
}
|
||||
|
||||
@Inject
|
||||
lateinit var db: AppDatabase
|
||||
@Inject
|
||||
lateinit var settings: SettingsManager
|
||||
|
||||
private var initialized = false
|
||||
|
||||
val cause = MutableLiveData<Throwable>()
|
||||
var logFile = MutableLiveData<File>()
|
||||
val localResource = MutableLiveData<String>()
|
||||
|
|
@ -243,32 +262,15 @@ class DebugInfoActivity : AppCompatActivity() {
|
|||
val zipFile = MutableLiveData<File>()
|
||||
val error = MutableLiveData<String>()
|
||||
|
||||
// private storage, not readable by others
|
||||
private val debugInfoDir = File(getApplication<Application>().filesDir, "debug")
|
||||
|
||||
init {
|
||||
// create debug info directory
|
||||
if (!debugInfoDir.isDirectory && !debugInfoDir.mkdir())
|
||||
throw IOException("Couldn't create debug info directory")
|
||||
}
|
||||
|
||||
@UiThread
|
||||
fun generate(extras: Bundle?) {
|
||||
if (initialized)
|
||||
return
|
||||
initialized = true
|
||||
val debugDir = Logger.debugDir() ?: throw IOException("Couldn't create debug info directory")
|
||||
|
||||
viewModelScope.launch(Dispatchers.Default) {
|
||||
val logFileName = extras?.getString(EXTRA_LOG_FILE)
|
||||
// create log file from EXTRA_LOGS or log file
|
||||
val logsText = extras?.getString(EXTRA_LOGS)
|
||||
if (logFileName != null) {
|
||||
val file = File(logFileName)
|
||||
if (file.isFile && file.canRead())
|
||||
logFile.postValue(file)
|
||||
else
|
||||
Logger.log.warning("Can't read logs from $logFileName")
|
||||
} else if (logsText != null) {
|
||||
val file = File(debugInfoDir, FILE_LOGS)
|
||||
if (logsText != null) {
|
||||
val file = File(debugDir, FILE_LOGS)
|
||||
if (!file.exists() || file.canWrite()) {
|
||||
file.writer().buffered().use { writer ->
|
||||
IOUtils.copy(StringReader(logsText), writer)
|
||||
|
|
@ -276,6 +278,9 @@ class DebugInfoActivity : AppCompatActivity() {
|
|||
logFile.postValue(file)
|
||||
} else
|
||||
Logger.log.warning("Can't write logs to $file")
|
||||
} else Logger.getDebugLogFile()?.let { debugLogFile ->
|
||||
if (debugLogFile.isFile && debugLogFile.canRead())
|
||||
logFile.postValue(debugLogFile)
|
||||
}
|
||||
|
||||
val throwable = extras?.getSerializable(EXTRA_CAUSE) as? Throwable
|
||||
|
|
@ -298,7 +303,7 @@ class DebugInfoActivity : AppCompatActivity() {
|
|||
}
|
||||
|
||||
private fun generateDebugInfo(syncAccount: Account?, syncAuthority: String?, cause: Throwable?, localResource: String?, remoteResource: String?) {
|
||||
val debugInfoFile = File(debugInfoDir, FILE_DEBUG_INFO)
|
||||
val debugInfoFile = File(Logger.debugDir(), FILE_DEBUG_INFO)
|
||||
debugInfoFile.writer().buffered().use { writer ->
|
||||
writer.append(ByteOrderMark.UTF_BOM)
|
||||
writer.append("--- BEGIN DEBUG INFO ---\n\n")
|
||||
|
|
@ -548,7 +553,7 @@ class DebugInfoActivity : AppCompatActivity() {
|
|||
try {
|
||||
zipProgress.postValue(true)
|
||||
|
||||
val file = File(debugInfoDir, "davx5-debug.zip")
|
||||
val file = File(Logger.debugDir(), "davx5-debug.zip")
|
||||
Logger.log.fine("Writing debug info to ${file.absolutePath}")
|
||||
ZipOutputStream(file.outputStream().buffered()).use { zip ->
|
||||
zip.setLevel(9)
|
||||
|
|
@ -723,7 +728,7 @@ class DebugInfoActivity : AppCompatActivity() {
|
|||
class IntentBuilder(context: Context) {
|
||||
|
||||
companion object {
|
||||
val MAX_ELEMENT_SIZE = 800 * FileUtils.ONE_KB.toInt()
|
||||
const val MAX_ELEMENT_SIZE = 800 * FileUtils.ONE_KB.toInt()
|
||||
}
|
||||
|
||||
val intent = Intent(context, DebugInfoActivity::class.java)
|
||||
|
|
@ -757,12 +762,6 @@ class DebugInfoActivity : AppCompatActivity() {
|
|||
return this
|
||||
}
|
||||
|
||||
fun withLogFile(logFile: File?): IntentBuilder {
|
||||
if (logFile != null)
|
||||
intent.putExtra(EXTRA_LOG_FILE, logFile.absolutePath)
|
||||
return this
|
||||
}
|
||||
|
||||
fun withLogs(logs: String?): IntentBuilder {
|
||||
if (logs != null)
|
||||
intent.putExtra(EXTRA_LOGS, StringUtils.abbreviate(logs, MAX_ELEMENT_SIZE))
|
||||
|
|
|
|||
Loading…
Reference in a new issue