-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
Harden asn1 oid loader to invalid inputs [3.0, 3.1] #23034
Conversation
In the event that a config file contains this sequence: ======= openssl_conf = openssl_init config_diagnostics = 1 [openssl_init] oid_section = oids [oids] testoid1 = 1.2.3.4.1 testoid2 = A Very Long OID Name, 1.2.3.4.2 testoid3 = ,1.2.3.4.3 ====== The leading comma in testoid3 can cause a heap buffer overflow, as the parsing code will move the string pointer back 1 character, thereby pointing to an invalid memory space correct the parser to detect this condition and handle it by treating it as if the comma doesn't exist (i.e. an empty long oid name) Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#22957) (cherry picked from commit a552c23)
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.
LGTM; faithful backport
Thank you for the reviews, but I'm concerned about the failing CI. It doesn't seem related, but appears consistently |
24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually. |
Merged to the 3.1 and 3.0 branches. Thank you. |
In the event that a config file contains this sequence: ======= openssl_conf = openssl_init config_diagnostics = 1 [openssl_init] oid_section = oids [oids] testoid1 = 1.2.3.4.1 testoid2 = A Very Long OID Name, 1.2.3.4.2 testoid3 = ,1.2.3.4.3 ====== The leading comma in testoid3 can cause a heap buffer overflow, as the parsing code will move the string pointer back 1 character, thereby pointing to an invalid memory space correct the parser to detect this condition and handle it by treating it as if the comma doesn't exist (i.e. an empty long oid name) (cherry picked from commit a552c23) Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #23034)
In the event that a config file contains this sequence: ======= openssl_conf = openssl_init config_diagnostics = 1 [openssl_init] oid_section = oids [oids] testoid1 = 1.2.3.4.1 testoid2 = A Very Long OID Name, 1.2.3.4.2 testoid3 = ,1.2.3.4.3 ====== The leading comma in testoid3 can cause a heap buffer overflow, as the parsing code will move the string pointer back 1 character, thereby pointing to an invalid memory space correct the parser to detect this condition and handle it by treating it as if the comma doesn't exist (i.e. an empty long oid name) (cherry picked from commit a552c23) Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #23034) (cherry picked from commit d802bfb)
In the event that a config file contains this sequence: ======= openssl_conf = openssl_init config_diagnostics = 1 [openssl_init] oid_section = oids [oids] testoid1 = 1.2.3.4.1 testoid2 = A Very Long OID Name, 1.2.3.4.2 testoid3 = ,1.2.3.4.3 ====== The leading comma in testoid3 can cause a heap buffer overflow, as the parsing code will move the string pointer back 1 character, thereby pointing to an invalid memory space correct the parser to detect this condition and handle it by treating it as if the comma doesn't exist (i.e. an empty long oid name) (cherry picked from commit a552c23) Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#23034) (cherry picked from commit d802bfb)
The new test broke
|
No this is yet another issue. Fix is in #23086. |
In the event that a config file contains this sequence: ======= openssl_conf = openssl_init config_diagnostics = 1 [openssl_init] oid_section = oids [oids] testoid1 = 1.2.3.4.1 testoid2 = A Very Long OID Name, 1.2.3.4.2 testoid3 = ,1.2.3.4.3 ====== The leading comma in testoid3 can cause a heap buffer overflow, as the parsing code will move the string pointer back 1 character, thereby pointing to an invalid memory space correct the parser to detect this condition and handle it by treating it as if the comma doesn't exist (i.e. an empty long oid name) (cherry picked from commit a552c23c6502592c1b3c67d93dd7e5ffbe958aa4) Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl/openssl#23034) Signed-off-by: lanming1120 <lanming1120@126.com>
In the event that a config file contains this sequence: ======= openssl_conf = openssl_init config_diagnostics = 1 [openssl_init] oid_section = oids [oids] testoid1 = 1.2.3.4.1 testoid2 = A Very Long OID Name, 1.2.3.4.2 testoid3 = ,1.2.3.4.3 ====== The leading comma in testoid3 can cause a heap buffer overflow, as the parsing code will move the string pointer back 1 character, thereby pointing to an invalid memory space correct the parser to detect this condition and handle it by treating it as if the comma doesn't exist (i.e. an empty long oid name) (cherry picked from commit a552c23c6502592c1b3c67d93dd7e5ffbe958aa4) Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl/openssl#23034) (cherry picked from commit d802bfbf80bab00123a4a6209f255852b3a10207) Signed-off-by: lanming1120 <lanming1120@126.com>
In the event that a config file contains this sequence:
The leading comma in testoid3 can cause a heap buffer overflow, as the parsing code will move the string pointer back 1 character, thereby pointing to an invalid memory space
correct the parser to detect this condition and handle it by treating it as if the comma doesn't exist (i.e. an empty long oid name)
Reviewed-by: Hugo Landau hlandau@openssl.org
Reviewed-by: Tomas Mraz tomas@openssl.org
(Merged from #22957)
(cherry picked from commit a552c23)
Checklist