Tools/pm 3567 import xxe detection (#6918)

* RegEx to prevent external entities from being imported in xml

* Adding the test case

* Changing the regex and updating test case description
This commit is contained in:
ttalty 2023-12-08 09:50:02 -05:00 committed by GitHub
parent 31112d8033
commit c4b31c9f8f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 27 additions and 0 deletions

View File

@ -12,6 +12,10 @@ class FakeBaseImporter extends BaseImporter {
setCardExpiration(cipher: CipherView, expiration: string): boolean {
return super.setCardExpiration(cipher, expiration);
}
parseXml(data: string): Document {
return super.parseXml(data);
}
}
describe("BaseImporter class", () => {
@ -103,5 +107,18 @@ describe("BaseImporter class", () => {
expect(result).toBe(false);
},
);
it("parse XML should reject xml with external entities", async () => {
const xml = `<?xml version="1.0" encoding="ISO-8859-1"?>
<!DOCTYPE replace [
<!ELEMENT replace ANY>
<!ENTITY xxe "External entity">
]>
<passwordsafe delimiter=";">
<entry><title>PoC XXE</title><username>&xxe;</username></entry>
</passwordsafe>`;
const result = importer.parseXml(xml);
expect(result).toBe(null);
});
});
});

View File

@ -137,6 +137,10 @@ export abstract class BaseImporter {
}
protected parseXml(data: string): Document {
// Ensure there are no external entity elements in the XML to prevent against XXE attacks.
if (!this.validateNoExternalEntities(data)) {
return null;
}
const parser = new DOMParser();
const doc = parser.parseFromString(data, "application/xml");
return doc != null && doc.querySelector("parsererror") == null ? doc : null;
@ -402,4 +406,10 @@ export abstract class BaseImporter {
cipher.identity.lastName = nameParts.slice(2, nameParts.length).join(" ");
}
}
private validateNoExternalEntities(data: string): boolean {
const regex = new RegExp("<!ENTITY", "i");
const hasExternalEntities = regex.test(data);
return !hasExternalEntities;
}
}