| Dec |
JAN |
Feb |
|
23 |
|
| 2025 |
2026 |
2027 |
About this capture
The Wayback Machine - http://web.archive.org/web/20260123174251/https://github.com/github/codeql/pull/4774
Skip to content
Navigation Menu
Appearance settings
AI CODE CREATION
GitHub CopilotWrite better code with AI
GitHub SparkBuild and deploy intelligent apps
GitHub ModelsManage and compare prompts
MCP RegistryNewIntegrate external tools
DEVELOPER WORKFLOWS
ActionsAutomate any workflow
CodespacesInstant dev environments
IssuesPlan and track work
Code ReviewManage code changes
APPLICATION SECURITY
GitHub Advanced SecurityFind and fix vulnerabilities
Code securitySecure your code as you build
Secret protectionStop leaks before they start
EXPLORE
●Why GitHub
●Documentation
●Blog
●Changelog
●Marketplace
View all features
BY COMPANY SIZE
●Enterprises
●Small and medium teams
●Startups
●Nonprofits
BY USE CASE
●App Modernization
●DevSecOps
●DevOps
●CI/CD
●View all use cases
BY INDUSTRY
●Healthcare
●Financial services
●Manufacturing
●Government
●View all industries
View all solutions
EXPLORE BY TOPIC
●AI
●Software Development
●DevOps
●Security
●View all topics
EXPLORE BY TYPE
●Customer stories
●Events & webinars
●Ebooks & reports
●Business insights
●GitHub Skills
SUPPORT & SERVICES
●Documentation
●Customer support
●Community forum
●Trust center
●Partners
COMMUNITY
GitHub SponsorsFund open source developers
PROGRAMS
●Security Lab
●Maintainer Community
●Accelerator
●Archive Program
REPOSITORIES
●Topics
●Trending
●Collections
ENTERPRISE SOLUTIONS
Enterprise platformAI-powered developer platform
AVAILABLE ADD-ONS
GitHub Advanced SecurityEnterprise-grade security features
Copilot for BusinessEnterprise-grade AI features
Premium SupportEnterprise-grade 24/7 support
●Pricing
Search or jump to...
Clear
Search syntax tips
Provide feedback
Saved searches
Use saved searches to filter your results more quickly
To see all available qualifiers, see our documentation.
Appearance settings
You signed in with another tab or window. Reload to refresh your session.
You signed out in another tab or window. Reload to refresh your session.
You switched accounts on another tab or window. Reload to refresh your session.
Dismiss alert
{{ message }}
github
/
codeql
Public
●
Notifications
You must be signed in to change notification settings
●
Fork
1.9k
Star
9.2k
●
Code
●
Issues
919
●
Pull requests
358
●
Discussions
●
Actions
●
Projects
0
●
Models
●
Security
0
●
Insights
Additional navigation options
●
Code
●
Issues
●
Pull requests
●
Discussions
●
Actions
●
Projects
●
Models
●
Security
●
Insights
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.
Sign up for GitHub
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
Jump to bottom
Merged
codeql-ci
merged 2 commits into
github:main
from
erik-krogh:forms
Jan 12, 2021
Merged
codeql-ci
merged 2 commits into
github:main
from
erik-krogh:forms
Jan 12, 2021
Files changed
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Show hidden characters
Contributor
Gets a TP/TN for CVE-2018-8035
We must have lost the TP with f23c603.
And this is my attempt at recognizing the source again.
Sorry, something went wrong.
github-actions
bot
added
the
JS
label
Dec 3, 2020
erik-krogh
added 2commits
December 3, 2020 16:57
better support for forms in js/xss-through-dom
3bad75d
update test
47488f8
erik-krogh
force-pushed
the
forms
branch
from
501fa8cto47488f8
Compare
December 3, 2020 15:58
erik-krogh
marked this pull request as ready for review
December 14, 2020 13:17
erik-krogh
requested a review
from a team
as a code owner
December 14, 2020 13:17
asgerf
reviewed
Dec 16, 2020
View reviewed changes
javascript/ql/src/semmle/javascript/DOM.qll
|
|
result = read
|
|
|
|
|
|
|
read.mayHavePropertyName(_) and
|
|
|
not read.mayHavePropertyName(getADomPropertyName())
|
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the addition of this rule it seems we now consider all properties of document to be DOM value sources.
Maybe we should just express that in a more direct way.
Sorry, something went wrong.
Contributor
Author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you suggesting that I put documentRef().getAPropertyRead(any(string s | not s = getADomPropertyName())) into DefaultRange?
I'm also using the forms predicate further down, so I would also like to keep the prop-read inside the forms predicate.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just saying that all of documentRef().getAPropertyRead() is now a DOM value source, but it's done in a very non-obvious, roundabout way, by unioning three sets:
●all prop reads with a DOM property name
●all prop reads with a non-DOM property name, and
●all property reads with an unknown property name.
If we end up with all of them anyway, I'd just prefer to have documentRef().getAPropertyRead() be listed as an explicit case in DefaultRange. It's fine to keep the forms() predicate.
Sorry, something went wrong.
Contributor
Author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documentRef().getAPropertyRead() is now a DOM value source
Hmmm. That turns out not to be the case.
I tried to add documentRef().getAPropertyRead(), but then the tests failed because document.location became a domValueRef.
domValueRef() does not contain documentRef(), so the property-read near the top of DefaultRange' does not refer to document`.
Sorry, something went wrong.
asgerf
reviewed
Jan 11, 2021
View reviewed changes
Contributor
asgerf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, LGTM then. Can you run a quick evaluation?
Sorry, something went wrong.
Contributor
Author
Sorry, something went wrong.
asgerf
approved these changes
Jan 12, 2021
View reviewed changes
codeql-ci
merged commit 1c8547c
into
github:main
Jan 12, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
asgerf
asgerf approved these changes
No one assigned
JS
No milestone
Successfully merging this pull request may close these issues.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Footer
© 2026 GitHub, Inc.
●
Terms
●
Privacy
●
Security
●
Status
●
Community
●
Docs
●
Contact
●
●
ation
You can’t perform that action at this time.
erik-krogh
commented
