Skip to content
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

Fix memory bug in grammar parser #7194

Merged
merged 1 commit into from
May 10, 2024
Merged

Fix memory bug in grammar parser #7194

merged 1 commit into from
May 10, 2024

Conversation

jart
Copy link
Contributor

@jart jart commented May 10, 2024

The llama.cpp grammar parser had a bug where forgetting to add a closing quotation mark to strings would cause parsing to crash. Anyone running a server on a public endpoint is advised to upgrade. To reproduce this bug

./llamafile -m foo.gguf -p bar --grammar 'root::="'

Credit for discovering and reporting this issue goes to Eclypsium Security Researcher Richard Johnson Richard.johnson@eclypsium.com.

jart added a commit to Mozilla-Ocho/llamafile that referenced this pull request May 10, 2024
This important change was accidentally removed in 94d0940. Credit for
discovering (and most importantly, reporting) this issue goes to
Eclypsium Security Researcher Richard Johnson.

Bug fix sent upstream in ggerganov/llama.cpp#7194
@mofosyne mofosyne added bugfix fixes an issue or bug Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix labels May 10, 2024
Copy link
Collaborator

@mofosyne mofosyne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Observations:

  • common/common.cpp : gpt_params_parse_ex() now throws an invalid parameter right at the point of of argument

  • common/grammar-parser.cpp : grammar_parser() now throws for when string unexpectedly end while searching for closing characters " or ] (as well as after - within square brackets)

  • examples/llava/llava-cli.cpp : process_prompt() will now exit(1) when llama_sampling_init() fails to initialize

  • examples/main/main.cpp : main() will now exit(1) when llama_sampling_init() fails to initialize


CI test failed, but unsure on cause of error. Subprocess aborted? Might be a CI fluke?

 18 - test-grammar-integration (Subprocess aborted)
 24 - test-json-schema-to-grammar (Subprocess aborted)

The llama.cpp grammar parser had a bug where forgetting to add a closing
quotation mark to strings would cause parsing to crash. Anyone running a
server on a public endpoint is advised to upgrade. To reproduce this bug

    ./llamafile -m foo.gguf -p bar --grammar 'root::="'

Credit for discovering and reporting this issue goes to Eclypsium
Security Researcher Richard Johnson <Richard.johnson@eclypsium.com>.
@mofosyne mofosyne merged commit 4e38809 into ggerganov:master May 10, 2024
57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix fixes an issue or bug Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants