A possible issue in the scanner when a LoadSyntax file is not found

[Gaiamap]

Hello everyone,

this is my first message here, so let me start by saying thank you. I have been reading the discussions in this group for some time, and I have found them genuinely interesting — both for what they explain about Ring and for the kind and patient tone in which things get discussed. It is a pleasant place to learn, and I am grateful to those who make it so.

I am writing because something came up while I was working on a project of mine, and I would like to submit it very quietly for your consideration. When a LoadSyntax directive names a file that cannot be found, the scanner appears to copy the full path it has just tried into a buffer that was sized for the short file name written in the source. In practice this can end with the process being aborted by the allocator, or with memory being quietly overwritten.

I have tried to check my own conclusion before bringing it here, rather than after. The report below contains a reproduction in three lines, the reason I believe it is an out-of-bounds write and not intended behaviour, and a one-line change that I have built and tested: it keeps the error message exactly as it is today, and 400 scripts from language/tests/scripts produce byte-identical output before and after, including all the ones that use LoadSyntax.

Two things I would like to be clear about. The first is that the technical report was drafted by an AI assistant that I have been working with on my project, and that I read through before posting; every claim in it was checked by running the code and by reading the sources, and the reproduction steps are included precisely so that nothing has to be taken on trust. The second is that I am well aware I may be missing context that you have and I do not — if the behaviour is known, or intended, or simply not worth the change, I will be glad to have learned something.

Thank you for your time and for the work behind this language.

With kind regards,

[Mahmoud Fayed]

Hello Gaiamap

Thanks for the report :D

After revising LoadSyntax command & your report, the next two issues are fixed

(1) LoadSyntax could get any string from Ring code, but then copy this string to a buffer suitable for path size (This size depends on the platform, Ring could use 4096 characters which is large enough, but nothing prevents Ring code from passing a larger size. Also, in some platforms this size could be 256 characters. 

So, we did a size check, and clear error message in this commit:  Update Visual Source (Using PWCT) - language/visualsrc/ring_scanner.s… · ring-lang/ring@51a43a3

(2) During processing Ring could copy Ring executable path & passed file name to the buffer it received that contains the file name alone, this is what your report contains and it's a clear/big mistake that we fixed in this commit:  Update Visual Source (Using PWCT) - language/visualsrc/ring_scanner.s… · ring-lang/ring@99f1f92

This error was not appearing in many platforms or when ring.exe path was small because Ring uses a memory pool and custom String implementation that already allocates extra size for strings (to improve the performance by reducing reallocations). 

Feel free to check again after this update and see if you still have any issues

Keep up the GREAT WORK

Greetings,

Mahmoud

[Gaiamap]

Hello Mahmoud,

thank you — both for the quick fix and for the explanation.

I have checked again as you suggested. Not being able to build the updated tree here, I applied your two changes to my local sources and rebuilt: the case that used to abort — an eight-character name with the interpreter in a 70-character path — now prints File: aaa.ring doesn't exist! and carries on, the twenty-character one behaves the same, and a 9000-character name is caught by the new size check with the E18 message. No crash in any of them. I will verify against the official build when it is available.

The note about the memory pool and the custom String implementation is the part I was missing: it explains exactly why the threshold looked so erratic in my tests — crashing with one name length and not with the next — which had puzzled me.

Thank you also for the second fix: I had not considered that the path buffer can be much smaller on other platforms.

With kind regards,

Cesare G. Fanelli (Gaiamap)

[Mahmoud Fayed]

Hello Cesare

>> "thank you — both for the quick fix and for the explanation."

Thanks again for the report, added to Ring Team as a tester:  The Ring Programming Language   

>> "Thank you also for the second fix: I had not considered that the path buffer can be much smaller on other platforms."

You are welcome :D

The size is defined here: ring/language/include/ring.h at master · ring-lang/ring

Greetings,

Mahmoud

[Mansour Ayouni]

Hello Cesear,

Thank you for your contribution and welcome to the Team!

All the best,

Mansour

--

---
You received this message because you are subscribed to the Google Groups "The Ring Programming Language" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ring-lang+...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/ring-lang/b303dd66-1627-4baa-8527-48da39f06404n%40googlegroups.com.

[Gaiamap]

Thank You Mansour and Mahmoud!

Cesare