Skip to content

DOM Related Sources and Sinks#198

Merged
tmbrbr merged 19 commits into
SAP:mainfrom
tmbrbr:dom-tainting
Feb 8, 2024
Merged

DOM Related Sources and Sinks#198
tmbrbr merged 19 commits into
SAP:mainfrom
tmbrbr:dom-tainting

Conversation

@tmbrbr

@tmbrbr tmbrbr commented Feb 1, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

There are quite a lot of updates in this one:

  • Improving taint flows through element attributes
  • Improving the HTML parser's taint propagation for attributes
  • Additional element attribute source
  • Source for elements which have been selected with various DOM queries
  • Additional sinks (e.g. element.after) when inserting elements into the DOM which have tainted content

Example of DOM selector sources:

var element = document.getElementById("content_by_id");
var tainted = element.getAttribute("test");

check_tainted(tainted);
check_taint_source(tainted, "document.getElementById");

Example of DOM insertion sinks:

let container = document.createElement("div");
let p = document.createElement("p");
container.appendChild(p);
let template = document.createElement("template");
template.innerHTML = String.tainted("<div test='helllo'>Content Here</div>");

p.after(template);

check_tainted(container.outerHTML);
check_taint_source(container.outerHTML, "manual taint source");

More examples are in the test_dom.html file.

tmbrbr added 16 commits February 1, 2024 15:39
@tmbrbr
Foxhound: adding HTML attribute sources
43a0931
@tmbrbr
Foxhound: add tainting flag to prevent recursion
42880ed
@tmbrbr
Foxhound: adding more DOM attribute propagation
582991b
@tmbrbr
Foxhound: call element method for url attributes
72ad6b3
@tmbrbr
Foxhound: add skeleton DOM test
7664a9a
@tmbrbr
Foxhound: fixing taint propagation for DOM attribute sources
ab8aa43
@tmbrbr
Foxhound: Adding TaintList class
3777d09
@tmbrbr
Foxhound: first attempt at tainting only after queryselectors
8ea29a6
@tmbrbr
Foxhound: DumpTaint for flows
f33b3d7
@tmbrbr
Foxhound: add ElementById source
86182ad
@tmbrbr
Foxhound: Propagate taint for HTML attribute parsing
154ed48
@tmbrbr
Foxhound: first attempt at node attachment sinks
2a2a958
@tmbrbr
Foxhound: add innerHTML as Taint Operation
0f150a4
@tmbrbr
Foxhound: Additional DOM element selector sources
d7c8bb8
@tmbrbr
Correcting source name
416e3b5
@tmbrbr
Adding more DOM tree sinks
96af46f
@tmbrbr tmbrbr self-assigned this Feb 1, 2024
@tmbrbr tmbrbr requested review from leeN and vladidx February 1, 2024 15:49
@tmbrbr tmbrbr added the enhancement New feature or request label Feb 1, 2024
tmbrbr
tmbrbr commented Feb 1, 2024
View reviewed changes
Comment thread dom/base/Element.cpp
@leeN

leeN commented Feb 7, 2024

Copy link
Copy Markdown
Collaborator

As I said during yesterday's call, the code looks good, and the tests look good as well!

I am currently running a test crawl, and it feels like there are more timeouts (which makes sense, as the number of flows explodes), but I have yet to encounter a Foxhound crash so far.

I suggest turning these off by default via the preferences and requiring the user to turn them on manually. Otherwise, if old settings are reused, stuff like XSS scanning will suffer (resource increase, timeouts, etc.).

To summarize: Looks good to me for merging 👍

E.g., for msn.com:

67 https://msn.com/
Redirected to www.msn.com
Exported 378 findings for https://msn.com

leeN
leeN approved these changes Feb 7, 2024
View reviewed changes

@leeN leeN left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tmbrbr tmbrbr merged commit 3887f79 into SAP:main Feb 8, 2024
@leeN leeN mentioned this pull request Jun 12, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leeN leeN leeN approved these changes

@vladidx vladidx Awaiting requested review from vladidx

@SoheilKhodayari SoheilKhodayari Awaiting requested review from SoheilKhodayari

Assignees

@tmbrbr tmbrbr

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tmbrbr @leeN