UBSan null pointer violation in ESIGN + Fix for Issue #1338
Oscar Yang <[email protected]> Thu, 13 Nov 2025 18:57:14 +0800
| Newsgroups | gmane.comp.encryption.cryptopp |
|---|---|
| Message-ID | <CADzyQwhgEWH8gWwUxCG-hPKe3eenXzyJTqxZoixMZ0-taAWz2Q@mail.gmail.com> |
--00000000000008ac90064377bf59
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Hello Crypto++ maintainers and community,
I'm scc, and I've discovered an UndefinedBehaviorSanitizer (UBSan)
violation in the ESIGN implementation while working on Issue #1338 (
https://github.com/weidai11/cryptopp/issues/1338). I've developed a
comprehensive fix and validation infrastructure that I'd like to contribute=
.
*Issue Summary*
- Primary Issue: UBSan detects null pointer passed to memcpy in
esign.cpp:118 when an empty seed (size=3D0) is provided to
InvertibleESIGNFunction::GenerateRandom.
- Secondary Issue: This fix also resolves Issue #1338 (clang22 fortify
warning) by using .data() and an explicit bounds assertion.
*Environment*
- Crypto++ Version: Current master (commit 60f81a77)
- Operating System: Ubuntu (GitHub Actions: ubuntu-latest)
- Compiler: Clang (default) and Clang 22
- Build Flags: -fsanitize=3Daddress,undefined for sanitizer testing; -O3
-D_FORTIFY_SOURCE=3D2 for fortify validation
*Exact Error Message*
esign.cpp:118:25: runtime error: null pointer passed as argument 2,
which is declared to never be null
/usr/include/string.h:44:28: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior esign.cpp:118:25
*Stack Trace*
#0 in CryptoPP::InvertibleESIGNFunction::GenerateRandom(...)
/home/runner/work/cryptopp/cryptopp/esign.cpp:118:3
#1 in run_one(unsigned long, unsigned int)
/home/runner/work/cryptopp/cryptopp/tests/esign_memcpy_asan.cpp:30:9
#2 in main
/home/runner/work/cryptopp/cryptopp/tests/esign_memcpy_asan.cpp:47:17
*Root Cause*
When seedParam.size() =3D=3D 0, seedParam.begin() may return a null pointer=
,
which violates the C++ standard requirement that memcpy must receive
non-null pointers even when the copy size is 0.
*Minimal Reproduction*
A focused test exercising this path is available here:
https://github.com/scc-tw/cryptopp/blob/asan-esign-test/tests/esign_memcpy_=
asan.cpp
The test validates 17 seed lengths (including 0) across 3 modulus sizes for
comprehensive edge case coverage.
*Evidence*
Commit demonstrating the issue:
-
https://github.com/scc-tw/cryptopp/commit/f47507e6beb94c97cdda84f0d0d06f429=
9cc51d9
- CI failure:
https://github.com/scc-tw/cryptopp/actions/runs/19327409311/job/55281830058
Commit with UBSan fix:
-
https://github.com/scc-tw/cryptopp/commit/db3951aa00258c2789c2eab519b312de6=
157f948
- CI success:
https://github.com/scc-tw/cryptopp/actions/runs/19327600421/job/55282868767
Complete fix (UBSan + Issue #1338):
-
https://github.com/scc-tw/cryptopp/commit/ab1878dde7658ff8f18482d9944f6996b=
f3155a6
- CI validation:
https://github.com/scc-tw/cryptopp/actions/runs/19327831662/job/55283667912
*Pull Request #1340 <https://github.com/weidai11/cryptopp/pull/1340>*
I've opened Pull Request #1340 with this fix:
https://github.com/weidai11/cryptopp/pull/1340
The fix applies three changes to esign.cpp:117-122:
```cpp
seed.resize(seedParam.size() + 4);
// Help static analyzer verify bounds (Issue #1338)
CRYPTOPP_ASSERT(seed.size() >=3D seedParam.size() + 4);
// Guard against null pointer when size is 0 (UBSan)
if (seedParam.size() > 0)
std::memcpy(seed.data() + 4, seedParam.begin(), seedParam.size());
```
*Key changes:*
1. Use seed.data() instead of seed + 4: Clearer for static analyzers,
resolves clang22 fortify warning
2. Add CRYPTOPP_ASSERT: Explicit bounds verification helps compiler verify
safety
3. Guard with if (size > 0): Prevents null pointer UB, handles empty seed
edge case
This approach is inspired by the cryptopp-modern fork fix (commit 79ecd1f0
<https://github.com/Coralesoft/cryptopp-modern/commit/79ecd1f0>) with
additional UBSan protection.
*Testing*
The fix has been validated with:
- =E2=9C=85 ASan/UBSan (no violations)
- =E2=9C=85 Clang22 -O3 with fortify (no warnings)
- =E2=9C=85 Full cryptest.exe validation suite
- =E2=9C=85 51 ESIGN edge cases (0-128 byte seeds, 96-384 bit moduli)
*Questions*
1. Is the fix approach acceptable? (seed.data() + assert + size guard)
2. Would you like CI validation infrastructure (ASan/UBSan + clang22
fortify) added to the repository?
3. Any additional testing or changes needed for the PR?
I'm happy to make any adjustments based on your feedback. Thank you for
maintaining this excellent library!
Best regards,
scc
--=20
*scc =E6=A5=8A=E5=BF=97=E7=92=BF **| Software Engineer*
*=E5=A5=A7 =E7=BE=A9 =E6=99=BA =E6=85=A7 =E7=A7=91 =E6=8A=80*
*E* [email protected]
*T* +886-2-7739-0077
*F* +886-2-7739-0079
=E6=96=B0=E5=8C=97=E5=B8=82=E6=9D=BF=E6=A9=8B=E5=8D=80=E9=81=A0=E6=9D=B1=E8=
=B7=AF3=E8=99=9F6=E6=A8=93
Official Site: CyCraft.com <http://www.cycraft.com/>
--=20
You received this message because you are subscribed to the Google Groups "=
Crypto++ Users" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to [email protected].
To view this discussion visit https://groups.google.com/d/msgid/cryptopp-us=
ers/CADzyQwhgEWH8gWwUxCG-hPKe3eenXzyJTqxZoixMZ0-taAWz2Q%40mail.gmail.com.
--00000000000008ac90064377bf59
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
<div dir=3D"ltr"><div><div>Hello Crypto++ maintainers and community,<br><br=
>I'm scc, and I've discovered an UndefinedBehaviorSanitizer (UBSan)=
violation in the ESIGN implementation while working on Issue #1338 (<a hre=
f=3D"https://github.com/weidai11/cryptopp/issues/1338" target=3D"_blank">ht=
tps://github.com/weidai11/cryptopp/issues/1338</a>). I've developed a c=
omprehensive fix and validation infrastructure that I'd like to contrib=
ute.<br><br><b>Issue Summary</b><br>- Primary Issue: UBSan detects null poi=
nter passed to memcpy in esign.cpp:118 when an empty seed (size=3D0) is pro=
vided to InvertibleESIGNFunction::GenerateRandom.<br>- Secondary Issue: Thi=
s fix also resolves Issue #1338 (clang22 fortify warning) by using .data() =
and an explicit bounds assertion.<br><br><b>Environment</b><br>- Crypto++ V=
ersion: Current master (commit 60f81a77)<br>- Operating System: Ubuntu (Git=
Hub Actions: ubuntu-latest)<br>- Compiler: Clang (default) and Clang 22<br>=
- Build Flags: -fsanitize=3Daddress,undefined for sanitizer testing; -O3 -D=
_FORTIFY_SOURCE=3D2 for fortify validation<br><br><b>Exact Error Message</b=
><br>esign.cpp:118:25: runtime error: null pointer passed as argument 2,<br=
>which is declared to never be null<br>/usr/include/string.h:44:28: note: n=
onnull attribute specified here<br><br>SUMMARY: UndefinedBehaviorSanitizer:=
undefined-behavior esign.cpp:118:25<br><br><b>Stack Trace</b><br>#0 in Cry=
ptoPP::InvertibleESIGNFunction::GenerateRandom(...)<br>=C2=A0 =C2=A0/home/r=
unner/work/cryptopp/cryptopp/esign.cpp:118:3<br>#1 in run_one(unsigned long=
, unsigned int)<br>=C2=A0 =C2=A0/home/runner/work/cryptopp/cryptopp/tests/e=
sign_memcpy_asan.cpp:30:9<br>#2 in main<br>=C2=A0 =C2=A0/home/runner/work/c=
ryptopp/cryptopp/tests/esign_memcpy_asan.cpp:47:17<br><br><b>Root Cause</b>=
<br>When seedParam.size() =3D=3D 0, seedParam.begin() may return a null poi=
nter, which violates the C++ standard requirement that memcpy must receive =
non-null pointers even when the copy size is 0.<br><br><b>Minimal Reproduct=
ion</b><br><br>A focused test exercising this path is available here:<br><a=
href=3D"https://github.com/scc-tw/cryptopp/blob/asan-esign-test/tests/esig=
n_memcpy_asan.cpp" target=3D"_blank">https://github.com/scc-tw/cryptopp/blo=
b/asan-esign-test/tests/esign_memcpy_asan.cpp</a><br><br>The test validates=
17 seed lengths (including 0) across 3 modulus sizes for comprehensive edg=
e case coverage.<br><br><b>Evidence</b><br><br>Commit demonstrating the iss=
ue:<br>-=C2=A0<a href=3D"https://github.com/scc-tw/cryptopp/commit/f47507e6=
beb94c97cdda84f0d0d06f4299cc51d9" target=3D"_blank">https://github.com/scc-=
tw/cryptopp/commit/f47507e6beb94c97cdda84f0d0d06f4299cc51d9</a><br>- CI fai=
lure:=C2=A0<a href=3D"https://github.com/scc-tw/cryptopp/actions/runs/19327=
409311/job/55281830058" target=3D"_blank">https://github.com/scc-tw/cryptop=
p/actions/runs/19327409311/job/55281830058</a><br><br>Commit with UBSan fix=
:<br>-=C2=A0<a href=3D"https://github.com/scc-tw/cryptopp/commit/db3951aa00=
258c2789c2eab519b312de6157f948" target=3D"_blank">https://github.com/scc-tw=
/cryptopp/commit/db3951aa00258c2789c2eab519b312de6157f948</a><br>- CI succe=
ss:=C2=A0<a href=3D"https://github.com/scc-tw/cryptopp/actions/runs/1932760=
0421/job/55282868767" target=3D"_blank">https://github.com/scc-tw/cryptopp/=
actions/runs/19327600421/job/55282868767</a><br><br>Complete fix (UBSan + I=
ssue #1338):<br>-=C2=A0<a href=3D"https://github.com/scc-tw/cryptopp/commit=
/ab1878dde7658ff8f18482d9944f6996bf3155a6" target=3D"_blank">https://github=
.com/scc-tw/cryptopp/commit/ab1878dde7658ff8f18482d9944f6996bf3155a6</a><br=
>- CI validation:=C2=A0<a href=3D"https://github.com/scc-tw/cryptopp/action=
s/runs/19327831662/job/55283667912" target=3D"_blank">https://github.com/sc=
c-tw/cryptopp/actions/runs/19327831662/job/55283667912</a><br><br><b>Pull R=
equest=C2=A0<a href=3D"https://github.com/weidai11/cryptopp/pull/1340" targ=
et=3D"_blank">#1340</a></b><br>I've opened Pull Request #1340 with this=
fix:<br><a href=3D"https://github.com/weidai11/cryptopp/pull/1340" target=
=3D"_blank">https://github.com/weidai11/cryptopp/pull/1340</a><br><br>The f=
ix applies three changes to esign.cpp:117-122:<br>```cpp<br>seed.resize(see=
dParam.size() + 4);<br>// Help static analyzer verify bounds (Issue #1338)<=
br>CRYPTOPP_ASSERT(seed.size() >=3D seedParam.size() + 4);<br>// Guard a=
gainst null pointer when size is 0 (UBSan)<br>if (seedParam.size() > 0)<=
br>=C2=A0 =C2=A0 std::memcpy(seed.data() + 4, seedParam.begin(), seedParam.=
size());<br>```<br><br><b>Key changes:</b><br>1. Use seed.data() instead of=
seed + 4: Clearer for static analyzers, resolves clang22 fortify warning<b=
r>2. Add CRYPTOPP_ASSERT: Explicit bounds verification helps compiler verif=
y safety<br>3. Guard with if (size > 0): Prevents null pointer UB, handl=
es empty seed edge case<br><br>This approach is inspired by the cryptopp-mo=
dern fork fix (commit=C2=A0<a href=3D"https://github.com/Coralesoft/cryptop=
p-modern/commit/79ecd1f0" target=3D"_blank">79ecd1f0</a>) with additional U=
BSan protection.<br><br><b>Testing</b><br>The fix has been validated with:<=
br>-=C2=A0=E2=9C=85=C2=A0ASan/UBSan (no violations)<br>-=C2=A0=E2=9C=85=C2=
=A0Clang22 -O3 with fortify (no warnings)<br>-=C2=A0=E2=9C=85=C2=A0Full cry=
ptest.exe validation suite<br>-=C2=A0=E2=9C=85=C2=A051 ESIGN edge cases (0-=
128 byte seeds, 96-384 bit moduli)<br><br><b>Questions</b><br>1. Is the fix=
approach acceptable? (seed.data() + assert + size guard)<br>2. Would you l=
ike CI validation infrastructure (ASan/UBSan + clang22 fortify) added to th=
e repository?<br>3. Any additional testing or changes needed for the PR?<br=
><br>I'm happy to make any adjustments based on your feedback. Thank yo=
u for maintaining this excellent library!<br><br>Best regards,<br>scc<br></=
div><div><br></div></div><div><br></div><span class=3D"gmail_signature_pref=
ix">-- </span><br><div dir=3D"ltr" class=3D"gmail_signature" data-smartmail=
=3D"gmail_signature"><div dir=3D"ltr"><div href=3D"http://WS_promo" style=
=3D"color:rgb(80,0,80);width:auto;padding-top:2px;font-size:10px;border-top=
:1px solid rgb(238,238,238);margin-top:18px;display:table;direction:ltr;lin=
e-height:normal;border-spacing:initial"><div style=3D"padding-top:2px"><b><=
img src=3D"https://tracy.srv.wisestamp.com/px/4915903341920256.png" alt=3D"=
" style=3D"font-size:small"></b></div></div><table cellpadding=3D"0" cellsp=
acing=3D"0" style=3D"border-collapse:collapse;font-family:Arial;line-height=
:1.15;color:rgb(0,0,0)"><tbody><tr><td style=3D"vertical-align:top;padding:=
0.01px 14px 0.01px 0.01px"><table cellpadding=3D"0" cellspacing=3D"0" style=
=3D"border-collapse:collapse;width:65px"><tbody><tr><td style=3D"padding:0.=
01px"><img src=3D"https://img.dlvr.ws/anonymous_user/958dda97-65e1-49cd-9a4=
1-82517853687e.png" height=3D"65" width=3D"65" style=3D"width:65px;vertical=
-align:initial;border-radius:0px;display:block;height:65px"></td></tr></tbo=
dy></table>=C2=A0=C2=A0</td><td height=3D"1" width=3D"0" style=3D"width:0px=
;padding:0.02px;border-right:0.5px solid rgb(189,189,189);height:1px;font-s=
ize:2pt"></td><td valign=3D"top" style=3D"padding:0.01px 0.01px 0.01px 14px=
;vertical-align:top"><table cellpadding=3D"0" cellspacing=3D"0" style=3D"bo=
rder-collapse:collapse"><tbody><tr><td style=3D"line-height:1.2;padding:0.0=
1px 0.01px 12px"><span style=3D"color:rgb(80,0,80)"><p dir=3D"ltr" style=3D=
"margin:0px;outline:none;color:rgb(33,33,33);line-height:1.8;padding-top:0p=
x"><span style=3D"color:rgb(34,28,53);vertical-align:baseline"><b><font siz=
e=3D"4" face=3D"arial, sans-serif">scc =E6=A5=8A=E5=BF=97=E7=92=BF=C2=A0</f=
ont></b><b><font size=3D"4" face=3D"arial, sans-serif">|=C2=A0</font><font =
face=3D"arial, sans-serif">Software Engineer</font></b></span></p><p dir=3D=
"ltr" style=3D"margin:0px;outline:none;color:rgb(33,33,33);line-height:1.5;=
background-color:transparent;border-width:initial;border-style:none;border-=
color:initial;padding:0px"><span style=3D"color:rgb(255,89,90);vertical-ali=
gn:baseline"><font face=3D"arial, sans-serif" size=3D"4"><b>=E5=A5=A7 =E7=
=BE=A9 =E6=99=BA =E6=85=A7 =E7=A7=91 =E6=8A=80</b></font></span></p><p dir=
=3D"ltr" style=3D"margin:0px;outline:none;color:rgb(33,33,33);line-height:1=
.8;background-color:transparent;border-width:initial;border-style:none;bord=
er-color:initial;padding:0px"><font face=3D"arial, sans-serif"><span style=
=3D"color:rgb(255,89,90)"><b>E</b></span><span style=3D"color:rgb(34,28,53)=
">=C2=A0</span></font><a href=3D"mailto:[email protected]" style=3D"co=
lor:rgb(17,85,204);font-family:arial,sans-serif" target=3D"_blank">oscar.ya=
[email protected]</a><span style=3D"color:rgb(34,28,53);font-family:arial,sans=
-serif"></span></p></span><span style=3D"color:rgb(80,0,80)"><p dir=3D"ltr"=
style=3D"background-color:transparent;margin:0px;outline:none;color:rgb(33=
,33,33);line-height:1.8;border-width:initial;border-style:none;border-color=
:initial;padding:0px"><font face=3D"arial, sans-serif"><span style=3D"color=
:rgb(255,89,90)"><b>T</b></span><span style=3D"color:rgb(34,28,53)">=C2=A0+=
886-2-7739-0077</span></font></p><p dir=3D"ltr" style=3D"background-color:t=
ransparent;margin:0px;outline:none;color:rgb(33,33,33);line-height:1.8;bord=
er-width:initial;border-style:none;border-color:initial;padding:0px"><font =
face=3D"arial, sans-serif"><span style=3D"color:rgb(255,89,90)"><b>F</b></s=
pan><span style=3D"color:rgb(34,28,53)">=C2=A0+886-2-7739-0079</span></font=
></p><p dir=3D"ltr" style=3D"background-color:transparent;margin:0px;outlin=
e:none;color:rgb(33,33,33);line-height:1.8;border-width:initial;border-styl=
e:none;border-color:initial;padding:0px"><span style=3D"color:rgb(0,0,0)"><=
font face=3D"arial, sans-serif">=E6=96=B0=E5=8C=97=E5=B8=82=E6=9D=BF=E6=A9=
=8B=E5=8D=80=E9=81=A0=E6=9D=B1=E8=B7=AF3=E8=99=9F6=E6=A8=93</font></span></=
p></span><p dir=3D"ltr" style=3D"background-color:transparent;margin:0px;ou=
tline:none;color:rgb(33,33,33);line-height:1.8;padding:0px;border-width:ini=
tial;border-style:none;border-color:initial"><span style=3D"color:rgb(34,28=
,53)"><font face=3D"arial, sans-serif">Official Site:=C2=A0</font></span><a=
href=3D"http://www.cycraft.com/" style=3D"color:rgb(17,85,204);font-family=
:arial,sans-serif" target=3D"_blank">CyCraft.com</a></p></td></tr></tbody><=
/table></td></tr></tbody></table></div></div></div>
<p></p>
-- <br />
You received this message because you are subscribed to the Google Groups &=
quot;Crypto++ Users" group.<br />
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to <a href=3D"mailto:[email protected]">cryp=
[email protected]</a>.<br />
To view this discussion visit <a href=3D"https://groups.google.com/d/msgid/=
cryptopp-users/CADzyQwhgEWH8gWwUxCG-hPKe3eenXzyJTqxZoixMZ0-taAWz2Q%40mail.g=
mail.com?utm_medium=3Demail&utm_source=3Dfooter">https://groups.google.com/=
d/msgid/cryptopp-users/CADzyQwhgEWH8gWwUxCG-hPKe3eenXzyJTqxZoixMZ0-taAWz2Q%=
40mail.gmail.com</a>.<br />
--00000000000008ac90064377bf59--