Closed
Bug 45692
Opened 25 years ago
Closed 25 years ago
browser crashes when running this signed script
Categories
(Core :: Security, defect, P1)
Tracking
()
VERIFIED
WORKSFORME
People
(Reporter: czhang, Assigned: jeff.dyer)
References
()
Details
(Keywords: crash, Whiteboard: [need info])
Attachments
(4 files)
|
308 bytes,
text/html
|
Details | |
|
9.06 KB,
patch
|
Details | Diff | Splinter Review | |
|
13.72 KB,
patch
|
Details | Diff | Splinter Review | |
|
8.30 KB,
patch
|
Details | Diff | Splinter Review |
load 7/17 NT build, you have to have my certificate to run the test case or you
can sign the script by yourself.
the code is:
-------------------
<HTML>
<SCRIPT>
netscape.security.PrivilegeManager.enablePrivilege("UniversalFileAccess");
var f = new java.io.File("myDir");
f.mkdir();
var e = f.exists();
if (e) {
e = f["delete"](); // since "delete" is a keyword
}
alert("Test " + (e ? "passes" : "FAILS"));
</SCRIPT>
</HTML>
----------------------
if signed script does not support UniversalFileAccess or there is any other
error in the script, then the error might need to be handled decently instead
of crashing the browser, by the way, this one is working on NS4.7
|
Reporter | |
Comment 1•25 years ago
|
||
Sorry, it is Security thing
Component: Browser-General → Security: General
|
||
Comment 4•25 years ago
|
||
LiveConnect security has not yet been implemented, which is probably why this is
crashing. Assigning this to Jeff Dyer. Jeff, Cathy created this testcase as a
signed script, but you might want to try it as ordinary unsigned HTML (delete the
EnablePrivilege line) and see if it's the LiveConnect call which causes the
crash.
Assignee: mstoltz → jeff.dyer
Target Milestone: --- → M19
I'll check it out. Cathy - could you post the test case on a public site or
attach it here. Thanks.
Status: NEW → ASSIGNED
|
|
Reporter | |
Comment 6•25 years ago
|
||
I see a security exception thrown, but no crash (Debug build of 2000061408).
This bug will be fixed when the Java plugin learns how to use
nsScriptSecurityManager to check permissions. No ETA yet.
|
Assignee | |
Comment 10•25 years ago
|
||
|
||
Comment 11•25 years ago
|
||
Jeff, if you work in my suggestions, below, I'll give you r=. I'll defer to
someone on liveconnect for a=
On 8 September 12:45:01, Jeff Dyer wrote:
> diff -u -r1.4 nsISecurityContext.h
> +
> + /**
> + * Get the origin associated with the context.
> + *
> + * @param buf -- Result buffer.
> + * @param len -- Buffer length.
> + * @return -- NS_OK if the codebase string was obtained.
> + * -- NS_FALSE otherwise.
> + */
> + NS_IMETHOD GetOrigin(char* buf, int len) = 0;
> +
> + /**
> + * Get the certificate associated with the context.
> + *
> + * @param buf -- Result buffer.
> + * @param len -- Buffer length.
> + * @return -- NS_OK if the codebase string was obtained.
> + * -- NS_FALSE otherwise.
> + */
> + NS_IMETHOD GetCertificateID(char* buf, int len) = 0;
So the caller must allocate the buffer, right? Say that in the
comments.
> Index: modules/oji/src/nsCSecurityContext.cpp
> + if(!strcmp(target,"UniversalJavaPermission")) {
Use nsCRT::strcmp() instead.
> + *bAllowedAccess = m_HasUniversalJavaPermission;
Check for null bAllowedAccess first.
> +NS_METHOD
> +nsCSecurityContext::GetOrigin(char* buf, int len)
> +{
> + nsIPrincipal* principal = NULL;
Check for nsnull buf.
Maybe use nsCOMPtr<nsIPrincipal> principal, that way you won't have to
NS_IF_RELEASE it.
> +
> + // Get the Script Security Manager.
> +
> + nsresult rv = NS_OK;
> + NS_WITH_SERVICE(nsIScriptSecurityManager, secMan,
> + NS_SCRIPTSECURITYMANAGER_PROGID, &rv)
> + if (NS_FAILED(rv) || !secMan) return FALSE;
> +
> +
> + secMan->GetSubjectPrincipal(&principal);
If you use nsCOMPtr<nsIPrincipal> you would say:
secMan->GetSubjectPrincipal(getter_AddRefs(principal));
> + char* origin;
> + codebase->GetOrigin(&origin);
> + if(len<=strlen(origin)) {
Again, nsCRT::strlen(origin)
> + strcpy(buf,origin);
Again, nsCRT::strcpy()
DOH! You need to nsCRT::free(origin), especially before any error case
returns.
> +NS_METHOD
> +nsCSecurityContext::GetCertificateID(char* buf, int len)
> +{
Check for null buf.
> + nsIPrincipal* principal = NULL;
As above, use nsCOMPtr here.
> + secMan->GetSubjectPrincipal(&principal);
As above, use getter_AddRefs() here.
> + char* certificate;
> + cprincipal->GetCertificateID(&certificate);
> +
> + if(len<=strlen(certificate)) {
As above, use nsCRT::strlen(). Basically, never use Clib functions.
There is invariably a ns version of each clib function.
DOH! You need to nsCRT::free(origin), especially before any error case
returns.
> + return FALSE;
> + }
> +
> + strcpy(buf,certificate);
nsCRT::strcpy()
|
|
Assignee | |
Comment 12•25 years ago
|
||
|
||
Comment 13•25 years ago
|
||
Mitchell, how critical are signed scripts for shipping? Need info to determine
whether this should be nsbeta3+ or not.
Whiteboard: [need info]
|
||
Comment 14•25 years ago
|
||
Worksforme. I'm not seeing a crash on Mac, WinNT or Linux branch 10/3 builds.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•