On 2024/05/20 13:30:43 Christopher Schultz wrote:
> Michael,
> 
> On 5/20/24 06:52, Michael Osipov wrote:
> > On 2024/05/17 15:11:58 Christopher Schultz wrote:
> >> Michael,
> >>
> >> On 5/17/24 03:42, Michael Osipov wrote:
> >>> On 2024/05/16 21:37:34 Christopher Schultz wrote:
> >>>> Michael,
> >>>>
> >>>> On 5/16/24 12:00, Michael Osipov wrote:
> >>>>> On 2024/05/16 15:55:04 Andy Arismendi wrote:
> >>>>>> Ok great! Thank you for taking the time and making the effort to look 
> >>>>>> into this Michael, much appreciated!
> >>>>>
> >>>>> Here is a dynamically linked, patched version until there is an 
> >>>>> official release: 
> >>>>> http://home.apache.org/~michaelo/issues/tomcat/openssl-crash/
> >>>>>
> >>>>> Please give it a try.
> >>>>
> >>>> Since you have produced a debug build of tcnative (and other
> >>>> components?) could you post the debug trace of the native stack?
> >>>
> >>> Unfortunately I can't. While I have the files with debug symbols I am 
> >>> limited by https://github.com/mturk/cmsc?tab=readme-ov-file#warning. I 
> >>> don't have a full blown Visual Studio installed.
> >>
> >> Okay. If you did build with VS, can you get a debug build with a backtrace?
> > 
> > Unfortunately not. Currently, I don't have the capacity to do so.
> > 
> >> I guess you already tracked the crash to openssl_fopen. When I did a
> >> decompile of the official binary, I can see the code but it's very
> >> difficult to read:
> >>
> >> void FUN_1800cccd0(char *param_1,char *param_2)
> >>
> >> {
> >>     char cVar1;
> >>     longlong lVar2;
> >>     int iVar3;
> >>     DWORD DVar4;
> >>     char *pcVar5;
> >>     FILE *pFVar6;
> >>     int *piVar7;
> >>     ulonglong uVar8;
> >>     uint uVar9;
> >>     int cbMultiByte;
> >>     undefined *puVar10;
> >>     undefined *puVar11;
> >>     uint uVar12;
> >>     undefined8 uStackY_80;
> >>     undefined auStackY_78 [32];
> >>     wchar_t local_48 [8];
> >>     ulonglong local_38;
> >>     undefined8 uStack_30;
> >>
> >>     uStack_30 = 0x1800ccce3;
> >>     local_38 = DAT_18033f868 ^ (ulonglong)local_48;
> >>     cVar1 = *param_1;
> >>     uVar12 = 0;
> >>     pcVar5 = param_1;
> >>     for (uVar9 = uVar12; (cVar1 != '\0' && (uVar9 < 0x80000000)); uVar9 =
> >> uVar9 + 1) {
> >>       pcVar5 = pcVar5 + 1;
> >>       cVar1 = *pcVar5;
> >>     }
> >>     cbMultiByte = (uVar9 & 0x7fffffff) + 1;
> >>     uStackY_80 = 0x1800ccd50;
> >>     iVar3 = 
> >> MultiByteToWideChar(0xfde9,8,param_1,cbMultiByte,(LPWSTR)0x0,0);
> >>     DVar4 = 8;
> >>     if (iVar3 < 1) {
> >>       uStackY_80 = 0x1800ccd5d;
> >>       DVar4 = GetLastError();
> >>       if (DVar4 == 0x3ec) {
> >>         uStackY_80 = 0x1800ccd84;
> >>         iVar3 =
> >> MultiByteToWideChar(0xfde9,0,param_1,cbMultiByte,(LPWSTR)0x0,0);
> >>         DVar4 = 0;
> >>         if (0 < iVar3) goto LAB_1800ccdac;
> >>       }
> >>       uStackY_80 = 0x1800ccd91;
> >>       DVar4 = GetLastError();
> >>       puVar10 = auStackY_78;
> >>       puVar11 = auStackY_78;
> >>       if (DVar4 != 0x459) goto LAB_1800cce89;
> >>     }
> >>     else {
> >> LAB_1800ccdac:
> >>       uVar8 = (longlong)iVar3 * 2 + 0xf;
> >>       if (uVar8 <= (ulonglong)((longlong)iVar3 * 2)) {
> >>         uVar8 = 0xffffffffffffff0;
> >>       }
> >>       uStackY_80 = 0x1800ccdd1;
> >>       lVar2 = -(uVar8 & 0xfffffffffffffff0);
> >>       *(int *)(&stack0xffffffffffffffb0 + lVar2) = iVar3;
> >>       *(wchar_t **)(&stack0xffffffffffffffa8 + lVar2) = (wchar_t
> >> *)((longlong)local_48 + lVar2);
> >>       *(undefined8 *)(auStackY_78 + lVar2 + -8) = 0x1800ccdf7;
> >>       iVar3 = MultiByteToWideChar(0xfde9,DVar4,param_1,cbMultiByte,
> >>                                   *(LPWSTR *)(&stack0xffffffffffffffa8 +
> >> lVar2),
> >>                                   *(int *)(&stack0xffffffffffffffb0 +
> >> lVar2));
> >>       puVar11 = auStackY_78 + lVar2;
> >>       if (iVar3 == 0) goto LAB_1800cce89;
> >>       cVar1 = *param_2;
> >>       pcVar5 = param_2;
> >>       for (; (cVar1 != '\0' && (uVar12 < 0x80000000)); uVar12 = uVar12 + 
> >> 1) {
> >>         pcVar5 = pcVar5 + 1;
> >>         cVar1 = *pcVar5;
> >>       }
> >>       *(undefined4 *)(&stack0xffffffffffffffb0 + lVar2) = 8;
> >>       *(wchar_t **)(&stack0xffffffffffffffa8 + lVar2) = local_48;
> >>       *(undefined8 *)(auStackY_78 + lVar2 + -8) = 0x1800cce4d;
> >>       iVar3 = MultiByteToWideChar(0xfde9,0,param_2,(uVar12 & 0x7fffffff) + 
> >> 1,
> >>                                   *(LPWSTR *)(&stack0xffffffffffffffa8 +
> >> lVar2),
> >>                                   *(int *)(&stack0xffffffffffffffb0 +
> >> lVar2));
> >>       puVar11 = auStackY_78 + lVar2;
> >>       if (iVar3 == 0) goto LAB_1800cce89;
> >>       *(undefined8 *)(auStackY_78 + lVar2 + -8) = 0x1800cce5d;
> >>       pFVar6 = _wfopen((wchar_t *)((longlong)local_48 + lVar2),local_48);
> >>       puVar11 = auStackY_78 + lVar2;
> >>       if (pFVar6 != (FILE *)0x0) goto LAB_1800cce89;
> >>       *(undefined8 *)(auStackY_78 + lVar2 + -8) = 0x1800cce6a;
> >>       piVar7 = _errno();
> >>       puVar10 = auStackY_78 + lVar2;
> >>       if (*piVar7 != 2) {
> >>         *(undefined8 *)(auStackY_78 + lVar2 + -8) = 0x1800cce78;
> >>         piVar7 = _errno();
> >>         puVar10 = auStackY_78 + lVar2;
> >>         puVar11 = auStackY_78 + lVar2;
> >>         if (*piVar7 != 9) goto LAB_1800cce89;
> >>       }
> >>     }
> >>     *(undefined8 *)(puVar10 + -8) = 0x1800ccda7;
> >>     fopen(param_1,param_2);
> >>     puVar11 = puVar10;
> >> LAB_1800cce89:
> >>     uVar8 = local_38 ^ (ulonglong)local_48;
> >>     *(undefined8 *)(puVar11 + -8) = 0x1800cce95;
> >>     FUN_180263660(uVar8);
> >>     return;
> >> }
> >>
> >> Thanks for helping to at least link it to this openssl source:
> >>
> >> https://github.com/openssl/openssl/blob/45f5d51b72a262bf85c4461fbded91485ce6b9da/crypto/o_fopen.c#L38
> >>
> >> Since libtcnative.dll is statically-linked, it doesn't even need a
> >> symbol table for internal calls so the openssl_fopen token is completely
> >> lost. Also, libtcnative contains all of TCN, APR, and OpenSSL. TCN
> >> doesn't make direct Win32 calls so that leaves ... all of APR and
> >> OpenSSL to search for this pattern of calls.
> >>
> >> Since you know where the fault is occurring, do you know the native
> >> call-trace being performed? I'd love to know which component along the
> >> way is not properly checking for NULL.
> > 
> > Yes, sure:
> > * 
> > https://github.com/apache/tomcat-native/blob/6a6a6b2395036c6a6cabb2b8af22aa329e438436/native/src/sslcontext.c#L711
> 
> So, 'file' is null on this line? If so, I guess the bug is in tcnative.

Yes, it is NULL. I don't think that the bug in necessarily in tcnative because 
othe functions in OpenSSL do support NULL and fail appropriately. I consider 
this either bad documenation or missing input validation.

> > * 
> > https://github.com/openssl/openssl/blob/45f5d51b72a262bf85c4461fbded91485ce6b9da/ssl/ssl_cert.c#L834
> > * 
> > https://github.com/openssl/openssl/blob/45f5d51b72a262bf85c4461fbded91485ce6b9da/crypto/o_fopen.c#L42
> 
> Where does the call go from BIO_read_filename to openssl_fopen?

Here: 
https://github.com/openssl/openssl/blob/45f5d51b72a262bf85c4461fbded91485ce6b9da/crypto/bio/bss_file.c#L267-L295

> > The NULL pointer gets passed around and strlen() chokes on it.
> > 
> > Please also read my upstream report: 
> > https://github.com/openssl/openssl/issues/24416
> 
> Yes, I'm getting ready to post to that and wanted more information, 
> since it doesn't look like the OpenSSL team cares about the report.

Yes, please.

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to