Socket Security: Obfuscated require

[karlhorky]

Hi there, first of all, thanks for this package!

I wanted to report the "Obfuscated require" security problem with [email protected] that is reported by @feross' new Socket Security platform:

Obfuscated require
SEVERITY: High
DESCRIPTION: Package accesses dynamic properties of require and may be obfuscating code execution.
SUGGESTION: The package should not access dynamic properties of module. Instead use import or require directly.

It seems to be in the first line of the dist/index.js file - maybe added by build scripts / packaging / bundler somehow

Even if this isn't a real security issue, it may be worth it to try to avoid generating this warning, to avoid users seeing it and either reporting it again or looking for an alternative.

[m-ahmadi]

Hi, thanks for your attention.

maybe added by build scripts / packaging / bundler somehow

That's exactly what's happening...

The dist/jalaali.js file is the output of browserify that's in this file.

jalaali-js/build-umd.js

Line 8 in a0a1001

execSync('browserify x.js -s jalaali -o dist/jalaali.js');

Do you know of any options in the browserify cli that generates the file without that part?

[karlhorky]

Maybe some of the Socket Security team have some tips on that, such as @bmeck, @101arrowz or @feross

[m-ahmadi]

I have to say, it seemed a bit odd to me because browserify is a creation of the somewhat same team.

(I understand there might be things I'm not taking into account regarding reasoning behind security related stuff)

[m-ahmadi]

@behrang

Hi,
I wanted to discuss something,
since version 14, node.js supports esm modules,
so in-theory, the source code of this project (mainly the index.js file) can be converted to esm modules,
it only means replacing module.exports = {...} with export function foo(){...}
this part is easy
the hard part is to think about backward compatibility concerns.

در این لحضه که دارم این متن رو می‌نویسم، اعتقادم اینه که اگر شما برداری syntax ماژول ها رو عوض کنید و تبدیلشون کنید به ESM، فکر نمیکنم مشکلی پیش بیاد ولی نمی‌تونم با قطعیت بگم (یه مدتی هست خیلی در جریان تغییرات وب نیستم)، مبنای این حرفم اینه مسائل زیره،

یک موضوعی که مهمه اینه که وقتی webpack یا این کتابخانه های بسته‌بندی کد (مثل rollup وامثالهم) بر می‌خورن به یک فایلی که این syntax جدید رو دارن چی میشه، که این فکر نمی‌کنم مشکلی باشه، متاسفانه الان نمی‌تونم خودم اینو تست کنم، ,ولی میشه یه فولدر درست کرد، بعد شما این سورس‌کد جلالی (index.js) رو بزاری توی فولدر، بعد کنارش یه فایل درست کرد توی یکی دوتا توابع جلالی رو با سینتکس جدید import کنه، بعد بریم مثلا وب‌پک یا رول‌آپ (یا vite، یا چندتا از این چیزا) نصب کنیم، و ببینیم چی میشه

فعلا غیر از این موضوع "پشتیبانی روبه‌عقب" مشکلی دیگری متصور نیستم برای اینکه syntax ماژول ها تغییر داده بشه.
باز این موضوع به تصمیم شما برمیگرده.
با احترام وسپاس🙏

[karlhorky]

@m-ahmadi by the way, the Socket Security page for the latest version of jalaali-js no longer reports a problem (see screenshot below).

So maybe this issue can be closed?

[m-ahmadi]

@karlhorky Well, that's a good thing, but the underlying problem is still need to dealt with, and that's whether to switch from cjs syntax to esm syntax. (and this decision is something I've been monitoring and thinking about my own projects for a couple of years). Obviously esm is the future (it's been the future since 2017 I remember) but the practicality of the matters is something else.
Anyways, I thank you for your attention again. 🙏

[m-ahmadi]

Here is something we can't do currently:

<script type="module">

import { toGregorian } from './node_modules/jalaali-js/...';

</script>

But let's say we create another file in jalaal-js/dist/ folder named jalaali.esm.js (like many other packages do)

Then above code becomes possible.
but predicated that doing so does not break other packages that depend on this package.

[m-ahmadi]

@behrang

[behrang]

Hey @m-ahmadi

Thanks for your comments. I'm open to the upgrade of the structure and using ES modules. We can even increase the major version so that anyone using this package will not get into problems, and can update to the latest version if they want to.

However, I'm very busy at the moment. If you can provide a PR, I appreciate it.