hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-25 16:55:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #13034 from geoffw0/swifttodos

Browse Source 
Swift: Delete some TODO comments
This commit is contained in:
Geoffrey White
2023-05-10 11:09:27 +01:00
committed by GitHub
parent f62bbf2d4c d7382053ca
commit 3587204403
8 changed files with 118 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
swift/ql/lib/codeql/swift/elements/decl/PrecedenceGroupDecl.qll
View File

@@ -1,7 +1,5 @@
private import codeql.swift.generated.decl.PrecedenceGroupDecl
class PrecedenceGroupDecl extends Generated::PrecedenceGroupDecl {
override string toString() {
result = "precedencegroup ..." // TODO: Once we extract the name we can improve this.
}
override string toString() { result = "precedencegroup ..." }
}

4
swift/ql/lib/codeql/swift/elements/expr/TupleElementExpr.qll
View File

@@ -1,7 +1,5 @@
private import codeql.swift.generated.expr.TupleElementExpr
class TupleElementExpr extends Generated::TupleElementExpr {
override string toString() {
result = "." + this.getIndex() // TODO: Can be improved once we extract the name
}
override string toString() { result = "." + this.getIndex() }
}

1
swift/ql/lib/codeql/swift/frameworks/StandardLibrary/FilePath.qll
View File

@@ -15,7 +15,6 @@ class FilePath extends StructDecl {
*/
private class FilePathSummaries extends SummaryModelCsv {
override predicate row(string row) {
// TODO: Properly model this class
row = ";FilePath;true;init(stringLiteral:);(String);;Argument[0];ReturnValue;taint"
}
}

1
swift/ql/lib/codeql/swift/frameworks/StandardLibrary/NsUrl.qll
View File

@@ -10,7 +10,6 @@ private import codeql.swift.dataflow.ExternalFlow
*/
private class NsUrlSummaries extends SummaryModelCsv {
override predicate row(string row) {
// TODO: Properly model this class
row = ";NSURL;true;init(string:);(String);;Argument[0];ReturnValue;taint"
}
}

1
swift/ql/lib/codeql/swift/frameworks/StandardLibrary/WebView.qll
View File

@@ -138,7 +138,6 @@ private class JsValueSummaries extends SummaryModelCsv {
";JSValue;true;toRange();;;Argument[-1];ReturnValue;taint",
";JSValue;true;toRect();;;Argument[-1];ReturnValue;taint",
";JSValue;true;toSize();;;Argument[-1];ReturnValue;taint",
// TODO: These models could use content flow to be more precise
";JSValue;true;atIndex(_:);;;Argument[-1];ReturnValue;taint",
";JSValue;true;defineProperty(_:descriptor:);;;Argument[1];Argument[-1];taint",
";JSValue;true;forProperty(_:);;;Argument[-1];ReturnValue;taint",

6
swift/ql/lib/codeql/swift/security/CleartextStoragePreferencesExtensions.qll
View File

@@ -63,10 +63,12 @@ private class NSUbiquitousKeyValueStore extends CleartextStoragePreferencesSink
* A more complicated case, this is a macOS-only way of writing to
* NSUserDefaults by modifying the `NSUserDefaultsController.values: Any`
* object via reflection (`perform(Selector)`) or the `NSKeyValueCoding`,
* `NSKeyValueBindingCreation` APIs. (TODO)
* `NSKeyValueBindingCreation` APIs.
*/
private class NSUserDefaultsControllerStore extends CleartextStoragePreferencesSink {
NSUserDefaultsControllerStore() { none() }
NSUserDefaultsControllerStore() {
none() // not yet implemented
}
override string getStoreName() { result = "the user defaults database" }
}

1
swift/ql/lib/codeql/swift/security/PathInjectionExtensions.qll
View File

@@ -36,7 +36,6 @@ private class DefaultPathInjectionSink extends PathInjectionSink {
private class DefaultPathInjectionBarrier extends PathInjectionBarrier {
DefaultPathInjectionBarrier() {
// This is a simplified implementation.
// TODO: Implement a complete path barrier when Guards are available.
exists(CallExpr starts, CallExpr normalize, DataFlow::Node validated |
starts.getStaticTarget().getName() = "starts(with:)" and
starts.getStaticTarget().getEnclosingDecl() instanceof FilePath and

150
swift/ql/test/query-tests/Security/CWE-611/testLibxmlXXE.swift
View File

@@ -1,18 +1,52 @@
// --- stubs ---
class NSObject {
}
class Data {
init<S>(_ elements: S) {}
func copyBytes(to: UnsafeMutablePointer<UInt8>, count: Int) {}
}
struct URL {
init?(string: String) {}
func appendingPathComponent(_ pathComponent: String) -> URL { return self }
}
extension String {
init(contentsOf: URL) {
let data = ""
self.init(data)
class FileManager : NSObject {
class var `default`: FileManager { get { return FileManager() } }
var homeDirectoryForCurrentUser: URL { get { return URL(string: "")! } }
}
class Bundle : NSObject {
class var main: Bundle { get { return Bundle() } }
func path(forResource name: String?, ofType ext: String?) -> String? { return "" }
}
struct FilePath {
init?(_ url: URL) { }
init(_ string: String) { }
}
struct FileDescriptor {
let rawValue: CInt
struct AccessMode : RawRepresentable {
var rawValue: CInt
static let readOnly = AccessMode(rawValue: 0)
}
static func open(
_ path: FilePath,
_ mode: FileDescriptor.AccessMode,
options: Int? = nil,
permissions: Int? = nil,
retryOnInterrupt: Bool = true
) throws -> FileDescriptor {
return FileDescriptor(rawValue: 0)
}
}
@@ -46,53 +80,93 @@ func xmlParseInNodeContext(_ node: xmlNodePtr!, _ data: UnsafePointer<CChar>!, _
func xmlCtxtReadMemory(_ ctxt: xmlParserCtxtPtr!, _ buffer: UnsafePointer<CChar>!, _ size: Int32, _ URL: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
func xmlCtxtReadFd(_ ctxt: xmlParserCtxtPtr!, _ fd: Int32, _ URL: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
func xmlCtxtReadIO(_ ctxt: xmlParserCtxtPtr!, _ ioread: xmlInputReadCallback!, _ ioclose: xmlInputCloseCallback!, _ ioctx: UnsafeMutableRawPointer!, _ URL: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
func xmlParseChunk(_ ctxt: xmlParserCtxtPtr!, _ chunk: UnsafePointer<CChar>?, _ size: Int32, _ terminate: Int32) -> Int32 { return 0 }
// --- tests ---
func sourcePtr() -> UnsafeMutablePointer<UInt8> { return UnsafeMutablePointer<UInt8>.allocate(capacity: 0) }
func sourceCharPtr() -> UnsafeMutablePointer<CChar> { return UnsafeMutablePointer<CChar>.allocate(capacity: 0) }
func getACtxt() -> xmlParserCtxtPtr {
return 0 as! xmlParserCtxtPtr
}
func test() {
let remotePtr = sourcePtr()
let remoteCharPtr = sourceCharPtr()
let safeCharPtr = UnsafeMutablePointer<CChar>.allocate(capacity: 1024)
safeCharPtr.initialize(repeating: 0, count: 1024)
let _ = xmlReadFile(remoteCharPtr, nil, 0) // NO XXE: external entities not enabled
let _ = xmlReadFile(remoteCharPtr, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=57
let _ = xmlReadFile(remoteCharPtr, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=57
let _ = xmlReadFile(remoteCharPtr, nil, Int32(XML_PARSE_NOENT.rawValue | XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=57
let _ = xmlReadFile(remoteCharPtr, nil, Int32(XML_PARSE_NOENT.rawValue | 0)) // $ hasXXE=57
let _ = xmlReadFile(remoteCharPtr, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=96
let _ = xmlReadFile(remoteCharPtr, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=96
let _ = xmlReadFile(remoteCharPtr, nil, Int32(XML_PARSE_NOENT.rawValue | XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=96
let _ = xmlReadFile(remoteCharPtr, nil, Int32(XML_PARSE_NOENT.rawValue | 0)) // $ hasXXE=96
let _ = xmlReadDoc(remotePtr, nil, nil, 0) // NO XXE: external entities not enabled
let _ = xmlReadDoc(remotePtr, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=56
let _ = xmlReadDoc(remotePtr, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=56
let _ = xmlReadDoc(remotePtr, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=95
let _ = xmlReadDoc(remotePtr, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=95
let _ = xmlCtxtReadFile(nil, remoteCharPtr, nil, 0) // NO XXE: external entities not enabled
let _ = xmlCtxtReadFile(nil, remoteCharPtr, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=57
let _ = xmlCtxtReadFile(nil, remoteCharPtr, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=57
let _ = xmlCtxtReadFile(nil, remoteCharPtr, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=96
let _ = xmlCtxtReadFile(nil, remoteCharPtr, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=96
let _ = xmlParseInNodeContext(nil, remoteCharPtr, -1, 0, nil) // NO XXE: external entities not enabled
let _ = xmlParseInNodeContext(nil, remoteCharPtr, -1, Int32(XML_PARSE_DTDLOAD.rawValue), nil) // $ hasXXE=57
let _ = xmlParseInNodeContext(nil, remoteCharPtr, -1, Int32(XML_PARSE_NOENT.rawValue), nil) // $ hasXXE=57
let _ = xmlParseInNodeContext(nil, remoteCharPtr, -1, Int32(XML_PARSE_DTDLOAD.rawValue), nil) // $ hasXXE=96
let _ = xmlParseInNodeContext(nil, remoteCharPtr, -1, Int32(XML_PARSE_NOENT.rawValue), nil) // $ hasXXE=96
let _ = xmlCtxtReadDoc(nil, remotePtr, nil, nil, 0) // NO XXE: external entities not enabled
let _ = xmlCtxtReadDoc(nil, remotePtr, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=56
let _ = xmlCtxtReadDoc(nil, remotePtr, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=56
let _ = xmlCtxtReadDoc(nil, remotePtr, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=95
let _ = xmlCtxtReadDoc(nil, remotePtr, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=95
let _ = xmlReadMemory(remoteCharPtr, -1, nil, nil, 0) // NO XXE: external entities not enabled
let _ = xmlReadMemory(remoteCharPtr, -1, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=57
let _ = xmlReadMemory(remoteCharPtr, -1, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=57
let _ = xmlReadMemory(remoteCharPtr, -1, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=96
let _ = xmlReadMemory(remoteCharPtr, -1, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=96
let _ = xmlCtxtReadMemory(nil, remoteCharPtr, -1, nil, nil, 0) // NO XXE: external entities not enabled
let _ = xmlCtxtReadMemory(nil, remoteCharPtr, -1, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=57
let _ = xmlCtxtReadMemory(nil, remoteCharPtr, -1, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=57
// TODO: We would need to model taint around `xmlParserCtxtPtr`, file descriptors, and `xmlInputReadCallback`
// to be able to alert on these methods. Not doing it for now because of the effort required vs the expected gain.
let _ = xmlCtxtUseOptions(nil, 0)
let _ = xmlCtxtUseOptions(nil, Int32(XML_PARSE_NOENT.rawValue))
let _ = xmlCtxtUseOptions(nil, Int32(XML_PARSE_DTDLOAD.rawValue))
let _ = xmlReadFd(0, nil, nil, 0)
let _ = xmlReadFd(0, nil, nil, Int32(XML_PARSE_NOENT.rawValue))
let _ = xmlReadFd(0, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue))
let _ = xmlCtxtReadFd(nil, 0, nil, nil, 0)
let _ = xmlCtxtReadFd(nil, 0, nil, nil, Int32(XML_PARSE_NOENT.rawValue))
let _ = xmlCtxtReadFd(nil, 0, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue))
let _ = xmlReadIO(nil, nil, nil, nil, nil, 0)
let _ = xmlReadIO(nil, nil, nil, nil, nil, Int32(XML_PARSE_NOENT.rawValue))
let _ = xmlReadIO(nil, nil, nil, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue))
let _ = xmlCtxtReadIO(nil, nil, nil, nil, nil, nil, 0)
let _ = xmlCtxtReadIO(nil, nil, nil, nil, nil, nil, Int32(XML_PARSE_NOENT.rawValue))
let _ = xmlCtxtReadIO(nil, nil, nil, nil, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue))
let _ = xmlCtxtReadMemory(nil, remoteCharPtr, -1, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=96
let _ = xmlCtxtReadMemory(nil, remoteCharPtr, -1, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=96
let ctxt1 = getACtxt()
if (xmlCtxtUseOptions(ctxt1, 0) == 0) { // NO XXE: external entities not enabled
let _ = xmlParseChunk(ctxt1, remoteCharPtr, 1024, 0)
}
let ctxt2 = getACtxt()
if (xmlCtxtUseOptions(ctxt2, Int32(XML_PARSE_NOENT.rawValue)) == 0) { // $ MISSING: hasXXE=96
let _ = xmlParseChunk(ctxt2, remoteCharPtr, 1024, 0)
}
let ctxt3 = getACtxt()
if (xmlCtxtUseOptions(ctxt3, Int32(XML_PARSE_DTDLOAD.rawValue)) == 0) { // $ MISSING: hasXXE=96
let _ = xmlParseChunk(ctxt3, remoteCharPtr, 1024, 0)
}
let ctxt4 = getACtxt()
if (xmlCtxtUseOptions(ctxt4, Int32(XML_PARSE_DTDLOAD.rawValue)) == 0) { // $ NO XXE: the input chunk isn't tainted
let _ = xmlParseChunk(ctxt4, safeCharPtr, 1024, 0)
}
let remotePath = FileManager.default.homeDirectoryForCurrentUser.appendingPathComponent("shared.xml")
let safePath = Bundle.main.path(forResource: "my", ofType: "xml")
let remoteFd = try! FileDescriptor.open(FilePath(remotePath)!, FileDescriptor.AccessMode.readOnly)
let safeFd = try! FileDescriptor.open(FilePath(safePath!), FileDescriptor.AccessMode.readOnly)
let _ = xmlReadFd(remoteFd.rawValue, nil, nil, 0) // NO XXE: external entities not enabled
let _ = xmlReadFd(remoteFd.rawValue, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ MISSING: hasXXE=146
let _ = xmlReadFd(remoteFd.rawValue, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ MISSING: hasXXE=146
let _ = xmlReadFd(safeFd.rawValue, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // NO XXE: the input file is trusted
let ctxt5 = getACtxt()
let _ = xmlCtxtReadFd(ctxt5, remoteFd.rawValue, nil, nil, 0) // NO XXE: external entities not enabled
let _ = xmlCtxtReadFd(ctxt5, remoteFd.rawValue, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ MISSING: hasXXE=146
let _ = xmlCtxtReadFd(ctxt5, remoteFd.rawValue, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ MISSING: hasXXE=146
let _ = xmlCtxtReadFd(ctxt5, safeFd.rawValue, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // NO XXE: the input file is trusted
let ctxt6 = getACtxt()
if (xmlCtxtUseOptions(ctxt6, Int32(XML_PARSE_NOENT.rawValue)) == 0) { // $ MISSING: hasXXE=146
let _ = xmlCtxtReadFd(ctxt6, remoteFd.rawValue, nil, nil, 0)
}
let _ = xmlReadIO(nil, nil, nil, nil, nil, 0) // NO XXE: external entities not enabled
let _ = xmlReadIO(nil, nil, nil, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ MISSING: hasXXE=?
let _ = xmlReadIO(nil, nil, nil, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ MISSING: hasXXE=?
let _ = xmlCtxtReadIO(nil, nil, nil, nil, nil, nil, 0) // NO XXE: external entities not enabled
let _ = xmlCtxtReadIO(nil, nil, nil, nil, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ MISSING: hasXXE=?
let _ = xmlCtxtReadIO(nil, nil, nil, nil, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ MISSING: hasXXE=?
}