Remove security-severity tag to java/random-used-once #7613
Conversation
Raised in #7601, this is one of the only .ql files that has a security-severity score but not the tag "security", including many other queries that live outside the `Security/` subdirectory. Besides this the only other files with this security-severity-but-no-security-tag combination are: ``` java/ql/src/Frameworks/JavaEE/EJB/EjbContainerInterference.ql java/ql/src/Frameworks/JavaEE/EJB/EjbFileIO.ql java/ql/src/Frameworks/JavaEE/EJB/EjbNative.ql java/ql/src/Frameworks/JavaEE/EJB/EjbReflection.ql java/ql/src/Frameworks/JavaEE/EJB/EjbSecurityConfiguration.ql java/ql/src/Frameworks/JavaEE/EJB/EjbSerialization.ql java/ql/src/Frameworks/JavaEE/EJB/EjbSetSocketOrUrlFactory.ql ``` Given their location I'm assuming these queries are disabled by default and likely shouldn't changed?
|
I think that the actual problem was that the query had been (incorrectly IMHO) assigned a CWE, and because of that a security-severity score was automatically assigned to it. I don't think this query actually models |
|
If the complaint in the query description (a less-than-even distribution) is true, then that would make random numbers easier to predict than they should be. That seems like a (perhaps mild) security issue to me. |
There was a problem hiding this comment.
Ok, I saw the readability and maintainability tags and assumed that this was a code quality matter, but after reading more about it I agree it may be a low-severity security issue (because I think this would be very difficult to exploit practically). I also think it shouldn't have a severity of 9.8, but I'm hesitant to change that because it'll probably be changed back the next time the automation runs.
|
Yeah that does seem dubious. @aschackmull do you have an opinion? |
|
I think the query likely does a decent job as a code quality query, but a poor job as a security query, so for that reason I think the simplest solution is to just consider it to be the former and not the latter. |
This leaves the Java query in the same state as its C# cousin.
smowton
changed the title
|
Changed this to remove the security-severity score, leaving the tagging in the same state as the C# query of the same name. |
Raised in #7601, this is one of the only .ql files that has a security-severity score but not the tag "security", including many other queries that live outside the
Security/subdirectory.Besides this the only other files with this security-severity-but-no-security-tag combination are:
Given their location I'm assuming these queries are disabled by default and likely shouldn't changed?