1619468 - load of value 999, which is not a valid value for type 'SecurityPropertyState' in src/security/manager/ssl/nsSiteSecurityService.cpp:98

Status: NEW

(Core :: Security: PSM, defect, P2)

Description

[Tyson Smith [:tsmith]]

This was found via try af7c3b591419. Full logs can be found here.

Marking as s-s since I'm not sure what the impact is in this case.

To enable this check add the following to your mozconfig:

ac_add_options --enable-undefined-sanitizer="enum"

src/security/manager/ssl/nsSiteSecurityService.cpp:98:13: runtime error: load of value 999, which is not a valid value for type 'SecurityPropertyState'
    #0 0x7f9a86439794 in ReadState src/security/manager/ssl/nsSiteSecurityService.cpp:98:13
    #1 0x7f9a86439794 in ParseHSTSState src/security/manager/ssl/nsSiteSecurityService.cpp:178:18
    #2 0x7f9a86439794 in SiteHSTSState::SiteHSTSState(nsTString<char> const&, mozilla::OriginAttributes const&, nsTString<char> const&) src/security/manager/ssl/nsSiteSecurityService.cpp:211:16
    #3 0x7f9a864460d9 in nsSiteSecurityService::HostHasHSTSEntry(nsTAutoStringN<char, 64ul> const&, bool, unsigned int, mozilla::OriginAttributes const&, bool*, bool*, SecurityPropertySource*) src/security/manager/ssl/nsSiteSecurityService.cpp:1370:11
    #4 0x7f9a8644563e in nsSiteSecurityService::IsSecureHost(unsigned int, nsTSubstring<char> const&, unsigned int, mozilla::OriginAttributes const&, bool*, SecurityPropertySource*, bool*) src/security/manager/ssl/nsSiteSecurityService.cpp:1537:7
    #5 0x7f9a86445120 in nsSiteSecurityService::IsSecureURI(unsigned int, nsIURI*, unsigned int, mozilla::OriginAttributes const&, bool*, unsigned int*, bool*) src/security/manager/ssl/nsSiteSecurityService.cpp:1306:10
    #6 0x7f9a86444dc9 in nsSiteSecurityService::IsSecureURIScriptable(unsigned int, nsIURI*, unsigned int, JS::Handle<JS::Value>, bool*, unsigned int*, JSContext*, unsigned char, bool*) src/security/manager/ssl/nsSiteSecurityService.cpp:1269:10
    #7 0x7f9a7c1e4a81 in NS_InvokeByIndex src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:106
    #8 0x7f9a7db4f7fb in Invoke src/js/xpconnect/src/XPCWrappedNative.cpp:1634:10
    #9 0x7f9a7db4f7fb in Call src/js/xpconnect/src/XPCWrappedNative.cpp:1175:19
    #10 0x7f9a7db4f7fb in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) src/js/xpconnect/src/XPCWrappedNative.cpp:1141:23
    #11 0x7f9a7db54812 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:947:10

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

These aren't attacker-controlled values, but we should fix this. What's going on is the implementation is saving enums to a file and then reading them back at startup. This test ensures we properly handle unexpected values (e.g. if the file gets corrupted or if there was a bug writing it out). Since the underlying type of the enum may not be large enough to hold any given unexpected value, this is undefined behavior.

Incidentally, we do this all over the place in PSM when reading configurations from prefs, and we should fix those too. One solution would be to give these enums concrete types of the same type that we're comparing against. Another would potentially be to cast the enum values and not the input values when doing the switch (inputValue) { case Enum::Value: ... }.