Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOM Related Sources and Sinks #198

Merged
merged 19 commits into from
Feb 8, 2024
Merged

DOM Related Sources and Sinks #198

merged 19 commits into from
Feb 8, 2024

Conversation

tmbrbr
Copy link
Contributor

@tmbrbr tmbrbr commented Feb 1, 2024

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 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
@leeN
Copy link
Collaborator

leeN commented Feb 7, 2024

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

Copy link
Collaborator

@leeN leeN left a comment

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
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants